From cd953a3f11a88fdeada0f48a29033b0087a04f17 Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Sat, 14 Dec 2024 09:46:16 +0000 Subject: [PATCH 01/32] add user_status_changes table --- coderd/database/dump.sql | 38 ++++++++++++++++ coderd/database/foreign_key_constraint.go | 1 + .../000279_user_status_changes.down.sql | 12 ++++++ .../000279_user_status_changes.up.sql | 43 +++++++++++++++++++ coderd/database/models.go | 8 ++++ coderd/database/unique_constraint.go | 1 + 6 files changed, 103 insertions(+) create mode 100644 coderd/database/migrations/000279_user_status_changes.down.sql create mode 100644 coderd/database/migrations/000279_user_status_changes.up.sql diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 50519485dc505..80f0507f59665 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -423,6 +423,23 @@ $$; COMMENT ON FUNCTION provisioner_tagset_contains(provisioner_tags tagset, job_tags tagset) IS 'Returns true if the provisioner_tags contains the job_tags, or if the job_tags represents an untagged provisioner and the superset is exactly equal to the subset.'; +CREATE FUNCTION record_user_status_change() RETURNS trigger + LANGUAGE plpgsql + AS $$ +BEGIN + IF OLD.status IS DISTINCT FROM NEW.status THEN + INSERT INTO user_status_changes ( + user_id, + new_status + ) VALUES ( + NEW.id, + NEW.status + ); + END IF; + RETURN NEW; +END; +$$; + CREATE FUNCTION remove_organization_member_role() RETURNS trigger LANGUAGE plpgsql AS $$ @@ -1395,6 +1412,15 @@ COMMENT ON COLUMN user_links.oauth_refresh_token_key_id IS 'The ID of the key us COMMENT ON COLUMN user_links.claims IS 'Claims from the IDP for the linked user. Includes both id_token and userinfo claims. '; +CREATE TABLE user_status_changes ( + id uuid DEFAULT gen_random_uuid() NOT NULL, + user_id uuid NOT NULL, + new_status user_status NOT NULL, + changed_at timestamp with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL +); + +COMMENT ON TABLE user_status_changes IS 'Tracks the history of user status changes'; + CREATE TABLE workspace_agent_log_sources ( workspace_agent_id uuid NOT NULL, id uuid NOT NULL, @@ -1983,6 +2009,9 @@ ALTER TABLE ONLY templates ALTER TABLE ONLY user_links ADD CONSTRAINT user_links_pkey PRIMARY KEY (user_id, login_type); +ALTER TABLE ONLY user_status_changes + ADD CONSTRAINT user_status_changes_pkey PRIMARY KEY (id); + ALTER TABLE ONLY users ADD CONSTRAINT users_pkey PRIMARY KEY (id); @@ -2093,6 +2122,10 @@ CREATE INDEX idx_tailnet_tunnels_dst_id ON tailnet_tunnels USING hash (dst_id); CREATE INDEX idx_tailnet_tunnels_src_id ON tailnet_tunnels USING hash (src_id); +CREATE INDEX idx_user_status_changes_changed_at ON user_status_changes USING btree (changed_at); + +CREATE INDEX idx_user_status_changes_user_id ON user_status_changes USING btree (user_id); + CREATE UNIQUE INDEX idx_users_email ON users USING btree (email) WHERE (deleted = false); CREATE UNIQUE INDEX idx_users_username ON users USING btree (username) WHERE (deleted = false); @@ -2235,6 +2268,8 @@ CREATE TRIGGER trigger_upsert_user_links BEFORE INSERT OR UPDATE ON user_links F CREATE TRIGGER update_notification_message_dedupe_hash BEFORE INSERT OR UPDATE ON notification_messages FOR EACH ROW EXECUTE FUNCTION compute_notification_message_dedupe_hash(); +CREATE TRIGGER user_status_change_trigger BEFORE UPDATE ON users FOR EACH ROW EXECUTE FUNCTION record_user_status_change(); + ALTER TABLE ONLY api_keys ADD CONSTRAINT api_keys_user_id_uuid_fkey FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; @@ -2367,6 +2402,9 @@ ALTER TABLE ONLY user_links ALTER TABLE ONLY user_links ADD CONSTRAINT user_links_user_id_fkey FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; +ALTER TABLE ONLY user_status_changes + ADD CONSTRAINT user_status_changes_user_id_fkey FOREIGN KEY (user_id) REFERENCES users(id); + ALTER TABLE ONLY workspace_agent_log_sources ADD CONSTRAINT workspace_agent_log_sources_workspace_agent_id_fkey FOREIGN KEY (workspace_agent_id) REFERENCES workspace_agents(id) ON DELETE CASCADE; diff --git a/coderd/database/foreign_key_constraint.go b/coderd/database/foreign_key_constraint.go index 669ab85f945bd..18c82b83750fa 100644 --- a/coderd/database/foreign_key_constraint.go +++ b/coderd/database/foreign_key_constraint.go @@ -50,6 +50,7 @@ const ( ForeignKeyUserLinksOauthAccessTokenKeyID ForeignKeyConstraint = "user_links_oauth_access_token_key_id_fkey" // ALTER TABLE ONLY user_links ADD CONSTRAINT user_links_oauth_access_token_key_id_fkey FOREIGN KEY (oauth_access_token_key_id) REFERENCES dbcrypt_keys(active_key_digest); ForeignKeyUserLinksOauthRefreshTokenKeyID ForeignKeyConstraint = "user_links_oauth_refresh_token_key_id_fkey" // ALTER TABLE ONLY user_links ADD CONSTRAINT user_links_oauth_refresh_token_key_id_fkey FOREIGN KEY (oauth_refresh_token_key_id) REFERENCES dbcrypt_keys(active_key_digest); ForeignKeyUserLinksUserID ForeignKeyConstraint = "user_links_user_id_fkey" // ALTER TABLE ONLY user_links ADD CONSTRAINT user_links_user_id_fkey FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; + ForeignKeyUserStatusChangesUserID ForeignKeyConstraint = "user_status_changes_user_id_fkey" // ALTER TABLE ONLY user_status_changes ADD CONSTRAINT user_status_changes_user_id_fkey FOREIGN KEY (user_id) REFERENCES users(id); ForeignKeyWorkspaceAgentLogSourcesWorkspaceAgentID ForeignKeyConstraint = "workspace_agent_log_sources_workspace_agent_id_fkey" // ALTER TABLE ONLY workspace_agent_log_sources ADD CONSTRAINT workspace_agent_log_sources_workspace_agent_id_fkey FOREIGN KEY (workspace_agent_id) REFERENCES workspace_agents(id) ON DELETE CASCADE; ForeignKeyWorkspaceAgentMetadataWorkspaceAgentID ForeignKeyConstraint = "workspace_agent_metadata_workspace_agent_id_fkey" // ALTER TABLE ONLY workspace_agent_metadata ADD CONSTRAINT workspace_agent_metadata_workspace_agent_id_fkey FOREIGN KEY (workspace_agent_id) REFERENCES workspace_agents(id) ON DELETE CASCADE; ForeignKeyWorkspaceAgentPortShareWorkspaceID ForeignKeyConstraint = "workspace_agent_port_share_workspace_id_fkey" // ALTER TABLE ONLY workspace_agent_port_share ADD CONSTRAINT workspace_agent_port_share_workspace_id_fkey FOREIGN KEY (workspace_id) REFERENCES workspaces(id) ON DELETE CASCADE; diff --git a/coderd/database/migrations/000279_user_status_changes.down.sql b/coderd/database/migrations/000279_user_status_changes.down.sql new file mode 100644 index 0000000000000..b75acbcafca46 --- /dev/null +++ b/coderd/database/migrations/000279_user_status_changes.down.sql @@ -0,0 +1,12 @@ +-- Drop the trigger first +DROP TRIGGER IF EXISTS user_status_change_trigger ON users; + +-- Drop the trigger function +DROP FUNCTION IF EXISTS record_user_status_change(); + +-- Drop the indexes +DROP INDEX IF EXISTS idx_user_status_changes_changed_at; +DROP INDEX IF EXISTS idx_user_status_changes_user_id; + +-- Drop the table +DROP TABLE IF EXISTS user_status_changes; diff --git a/coderd/database/migrations/000279_user_status_changes.up.sql b/coderd/database/migrations/000279_user_status_changes.up.sql new file mode 100644 index 0000000000000..545705aba50eb --- /dev/null +++ b/coderd/database/migrations/000279_user_status_changes.up.sql @@ -0,0 +1,43 @@ +CREATE TABLE user_status_changes ( + id uuid PRIMARY KEY DEFAULT gen_random_uuid(), + user_id uuid NOT NULL REFERENCES users(id), + new_status user_status NOT NULL, + changed_at timestamptz NOT NULL DEFAULT CURRENT_TIMESTAMP +); + +COMMENT ON TABLE user_status_changes IS 'Tracks the history of user status changes'; + +CREATE INDEX idx_user_status_changes_user_id ON user_status_changes(user_id); +CREATE INDEX idx_user_status_changes_changed_at ON user_status_changes(changed_at); + +INSERT INTO user_status_changes ( + user_id, + new_status, + changed_at +) +SELECT + id, + status, + created_at +FROM users +WHERE NOT deleted; + +CREATE FUNCTION record_user_status_change() RETURNS trigger AS $$ +BEGIN + IF OLD.status IS DISTINCT FROM NEW.status THEN + INSERT INTO user_status_changes ( + user_id, + new_status + ) VALUES ( + NEW.id, + NEW.status + ); + END IF; + RETURN NEW; +END; +$$ LANGUAGE plpgsql; + +CREATE TRIGGER user_status_change_trigger + BEFORE UPDATE ON users + FOR EACH ROW + EXECUTE FUNCTION record_user_status_change(); diff --git a/coderd/database/models.go b/coderd/database/models.go index 9ca80d119a502..886626a58b3f3 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -2968,6 +2968,14 @@ type UserLink struct { Claims UserLinkClaims `db:"claims" json:"claims"` } +// Tracks the history of user status changes +type UserStatusChange struct { + ID uuid.UUID `db:"id" json:"id"` + UserID uuid.UUID `db:"user_id" json:"user_id"` + NewStatus UserStatus `db:"new_status" json:"new_status"` + ChangedAt time.Time `db:"changed_at" json:"changed_at"` +} + // Visible fields of users are allowed to be joined with other tables for including context of other resources. type VisibleUser struct { ID uuid.UUID `db:"id" json:"id"` diff --git a/coderd/database/unique_constraint.go b/coderd/database/unique_constraint.go index f4470c6546698..b59cb0cbc8091 100644 --- a/coderd/database/unique_constraint.go +++ b/coderd/database/unique_constraint.go @@ -63,6 +63,7 @@ const ( UniqueTemplateVersionsTemplateIDNameKey UniqueConstraint = "template_versions_template_id_name_key" // ALTER TABLE ONLY template_versions ADD CONSTRAINT template_versions_template_id_name_key UNIQUE (template_id, name); UniqueTemplatesPkey UniqueConstraint = "templates_pkey" // ALTER TABLE ONLY templates ADD CONSTRAINT templates_pkey PRIMARY KEY (id); UniqueUserLinksPkey UniqueConstraint = "user_links_pkey" // ALTER TABLE ONLY user_links ADD CONSTRAINT user_links_pkey PRIMARY KEY (user_id, login_type); + UniqueUserStatusChangesPkey UniqueConstraint = "user_status_changes_pkey" // ALTER TABLE ONLY user_status_changes ADD CONSTRAINT user_status_changes_pkey PRIMARY KEY (id); UniqueUsersPkey UniqueConstraint = "users_pkey" // ALTER TABLE ONLY users ADD CONSTRAINT users_pkey PRIMARY KEY (id); UniqueWorkspaceAgentLogSourcesPkey UniqueConstraint = "workspace_agent_log_sources_pkey" // ALTER TABLE ONLY workspace_agent_log_sources ADD CONSTRAINT workspace_agent_log_sources_pkey PRIMARY KEY (workspace_agent_id, id); UniqueWorkspaceAgentMetadataPkey UniqueConstraint = "workspace_agent_metadata_pkey" // ALTER TABLE ONLY workspace_agent_metadata ADD CONSTRAINT workspace_agent_metadata_pkey PRIMARY KEY (workspace_agent_id, key); From ec16728f3677328a069f007b2145d0f48820fcf4 Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Sat, 14 Dec 2024 10:37:43 +0000 Subject: [PATCH 02/32] add GetUserStatusCountsByDay --- coderd/database/dbauthz/dbauthz.go | 7 ++ coderd/database/dbmem/dbmem.go | 9 ++ coderd/database/dbmetrics/querymetrics.go | 7 ++ coderd/database/dbmock/dbmock.go | 15 ++++ coderd/database/querier.go | 1 + coderd/database/queries.sql.go | 103 ++++++++++++++++++++++ coderd/database/queries/insights.sql | 68 ++++++++++++++ 7 files changed, 210 insertions(+) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 0a35667ed0178..29b3f9f5105e1 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -2421,6 +2421,13 @@ func (q *querier) GetUserNotificationPreferences(ctx context.Context, userID uui return q.db.GetUserNotificationPreferences(ctx, userID) } +func (q *querier) GetUserStatusCountsByDay(ctx context.Context, arg database.GetUserStatusCountsByDayParams) ([]database.GetUserStatusCountsByDayRow, error) { + if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil { + return nil, err + } + return q.db.GetUserStatusCountsByDay(ctx, arg) +} + func (q *querier) GetUserWorkspaceBuildParameters(ctx context.Context, params database.GetUserWorkspaceBuildParametersParams) ([]database.GetUserWorkspaceBuildParametersRow, error) { u, err := q.db.GetUserByID(ctx, params.OwnerID) if err != nil { diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index d3b7b3fb35f5f..a3678e7431f22 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -5669,6 +5669,15 @@ func (q *FakeQuerier) GetUserNotificationPreferences(_ context.Context, userID u return out, nil } +func (q *FakeQuerier) GetUserStatusCountsByDay(ctx context.Context, arg database.GetUserStatusCountsByDayParams) ([]database.GetUserStatusCountsByDayRow, error) { + err := validateDatabaseType(arg) + if err != nil { + return nil, err + } + + panic("not implemented") +} + func (q *FakeQuerier) GetUserWorkspaceBuildParameters(_ context.Context, params database.GetUserWorkspaceBuildParametersParams) ([]database.GetUserWorkspaceBuildParametersRow, error) { q.mutex.RLock() defer q.mutex.RUnlock() diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index 5df5c547a20d6..595df4464e691 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -1344,6 +1344,13 @@ func (m queryMetricsStore) GetUserNotificationPreferences(ctx context.Context, u return r0, r1 } +func (m queryMetricsStore) GetUserStatusCountsByDay(ctx context.Context, arg database.GetUserStatusCountsByDayParams) ([]database.GetUserStatusCountsByDayRow, error) { + start := time.Now() + r0, r1 := m.s.GetUserStatusCountsByDay(ctx, arg) + m.queryLatencies.WithLabelValues("GetUserStatusCountsByDay").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m queryMetricsStore) GetUserWorkspaceBuildParameters(ctx context.Context, ownerID database.GetUserWorkspaceBuildParametersParams) ([]database.GetUserWorkspaceBuildParametersRow, error) { start := time.Now() r0, r1 := m.s.GetUserWorkspaceBuildParameters(ctx, ownerID) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index 6b552fe5060ff..aabc4f6828302 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -2825,6 +2825,21 @@ func (mr *MockStoreMockRecorder) GetUserNotificationPreferences(arg0, arg1 any) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserNotificationPreferences", reflect.TypeOf((*MockStore)(nil).GetUserNotificationPreferences), arg0, arg1) } +// GetUserStatusCountsByDay mocks base method. +func (m *MockStore) GetUserStatusCountsByDay(arg0 context.Context, arg1 database.GetUserStatusCountsByDayParams) ([]database.GetUserStatusCountsByDayRow, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetUserStatusCountsByDay", arg0, arg1) + ret0, _ := ret[0].([]database.GetUserStatusCountsByDayRow) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetUserStatusCountsByDay indicates an expected call of GetUserStatusCountsByDay. +func (mr *MockStoreMockRecorder) GetUserStatusCountsByDay(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserStatusCountsByDay", reflect.TypeOf((*MockStore)(nil).GetUserStatusCountsByDay), arg0, arg1) +} + // GetUserWorkspaceBuildParameters mocks base method. func (m *MockStore) GetUserWorkspaceBuildParameters(arg0 context.Context, arg1 database.GetUserWorkspaceBuildParametersParams) ([]database.GetUserWorkspaceBuildParametersRow, error) { m.ctrl.T.Helper() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 620cc14b3fd26..36401598c8d40 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -289,6 +289,7 @@ type sqlcQuerier interface { GetUserLinkByUserIDLoginType(ctx context.Context, arg GetUserLinkByUserIDLoginTypeParams) (UserLink, error) GetUserLinksByUserID(ctx context.Context, userID uuid.UUID) ([]UserLink, error) GetUserNotificationPreferences(ctx context.Context, userID uuid.UUID) ([]NotificationPreference, error) + GetUserStatusCountsByDay(ctx context.Context, arg GetUserStatusCountsByDayParams) ([]GetUserStatusCountsByDayRow, error) GetUserWorkspaceBuildParameters(ctx context.Context, arg GetUserWorkspaceBuildParametersParams) ([]GetUserWorkspaceBuildParametersRow, error) // This will never return deleted users. GetUsers(ctx context.Context, arg GetUsersParams) ([]GetUsersRow, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 8fbb7c0b5be6c..3c902fe22f532 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3094,6 +3094,109 @@ func (q *sqlQuerier) GetUserLatencyInsights(ctx context.Context, arg GetUserLate return items, nil } +const getUserStatusCountsByDay = `-- name: GetUserStatusCountsByDay :many +WITH dates AS ( + -- Generate a series of dates between start and end + SELECT + day::date + FROM + generate_series( + date_trunc('day', $1::timestamptz), + date_trunc('day', $2::timestamptz), + '1 day'::interval + ) AS day +), +initial_statuses AS ( + -- Get the status of each user right before the start date + SELECT DISTINCT ON (user_id) + user_id, + new_status as status + FROM + user_status_changes + WHERE + changed_at < $1::timestamptz + ORDER BY + user_id, + changed_at DESC +), +relevant_changes AS ( + -- Get only the status changes within our date range + SELECT + date_trunc('day', changed_at)::date AS day, + user_id, + new_status as status + FROM + user_status_changes + WHERE + changed_at >= $1::timestamptz + AND changed_at <= $2::timestamptz +), +daily_status AS ( + -- Combine initial statuses with changes + SELECT + d.day, + COALESCE(rc.status, i.status) as status, + COALESCE(rc.user_id, i.user_id) as user_id + FROM + dates d + CROSS JOIN + initial_statuses i + LEFT JOIN + relevant_changes rc + ON + rc.day = d.day + AND rc.user_id = i.user_id +) +SELECT + day, + status, + COUNT(*) AS count +FROM + daily_status +WHERE + status IS NOT NULL +GROUP BY + day, + status +ORDER BY + day ASC, + status ASC +` + +type GetUserStatusCountsByDayParams struct { + StartTime time.Time `db:"start_time" json:"start_time"` + EndTime time.Time `db:"end_time" json:"end_time"` +} + +type GetUserStatusCountsByDayRow struct { + Day time.Time `db:"day" json:"day"` + Status UserStatus `db:"status" json:"status"` + Count int64 `db:"count" json:"count"` +} + +func (q *sqlQuerier) GetUserStatusCountsByDay(ctx context.Context, arg GetUserStatusCountsByDayParams) ([]GetUserStatusCountsByDayRow, error) { + rows, err := q.db.QueryContext(ctx, getUserStatusCountsByDay, arg.StartTime, arg.EndTime) + if err != nil { + return nil, err + } + defer rows.Close() + var items []GetUserStatusCountsByDayRow + for rows.Next() { + var i GetUserStatusCountsByDayRow + if err := rows.Scan(&i.Day, &i.Status, &i.Count); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const upsertTemplateUsageStats = `-- name: UpsertTemplateUsageStats :exec WITH latest_start AS ( diff --git a/coderd/database/queries/insights.sql b/coderd/database/queries/insights.sql index de107bc0e80c7..248edd89fac23 100644 --- a/coderd/database/queries/insights.sql +++ b/coderd/database/queries/insights.sql @@ -771,3 +771,71 @@ SELECT FROM unique_template_params utp JOIN workspace_build_parameters wbp ON (utp.workspace_build_ids @> ARRAY[wbp.workspace_build_id] AND utp.name = wbp.name) GROUP BY utp.num, utp.template_ids, utp.name, utp.type, utp.display_name, utp.description, utp.options, wbp.value; + +-- name: GetUserStatusCountsByDay :many +WITH dates AS ( + -- Generate a series of dates between start and end + SELECT + day::date + FROM + generate_series( + date_trunc('day', @start_time::timestamptz), + date_trunc('day', @end_time::timestamptz), + '1 day'::interval + ) AS day +), +initial_statuses AS ( + -- Get the status of each user right before the start date + SELECT DISTINCT ON (user_id) + user_id, + new_status as status + FROM + user_status_changes + WHERE + changed_at < @start_time::timestamptz + ORDER BY + user_id, + changed_at DESC +), +relevant_changes AS ( + -- Get only the status changes within our date range + SELECT + date_trunc('day', changed_at)::date AS day, + user_id, + new_status as status + FROM + user_status_changes + WHERE + changed_at >= @start_time::timestamptz + AND changed_at <= @end_time::timestamptz +), +daily_status AS ( + -- Combine initial statuses with changes + SELECT + d.day, + COALESCE(rc.status, i.status) as status, + COALESCE(rc.user_id, i.user_id) as user_id + FROM + dates d + CROSS JOIN + initial_statuses i + LEFT JOIN + relevant_changes rc + ON + rc.day = d.day + AND rc.user_id = i.user_id +) +SELECT + day, + status, + COUNT(*) AS count +FROM + daily_status +WHERE + status IS NOT NULL +GROUP BY + day, + status +ORDER BY + day ASC, + status ASC; From 0d97e82562141a77d6a6c76697a161c061a160c3 Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Sat, 14 Dec 2024 10:39:31 +0000 Subject: [PATCH 03/32] rename unused variable --- coderd/database/dbmem/dbmem.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index a3678e7431f22..a4244544ee5c8 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -5669,7 +5669,10 @@ func (q *FakeQuerier) GetUserNotificationPreferences(_ context.Context, userID u return out, nil } -func (q *FakeQuerier) GetUserStatusCountsByDay(ctx context.Context, arg database.GetUserStatusCountsByDayParams) ([]database.GetUserStatusCountsByDayRow, error) { +func (q *FakeQuerier) GetUserStatusCountsByDay(_ context.Context, arg database.GetUserStatusCountsByDayParams) ([]database.GetUserStatusCountsByDayRow, error) { + q.mutex.RLock() + defer q.mutex.RUnlock() + err := validateDatabaseType(arg) if err != nil { return nil, err From eb6e2498855f34875003abc67c587f7024bf16e8 Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Tue, 17 Dec 2024 08:09:28 +0000 Subject: [PATCH 04/32] Test GetUserStatusCountsByDay --- coderd/database/dump.sql | 10 +- .../000279_user_status_changes.up.sql | 13 +- coderd/database/querier_test.go | 446 ++++++++++++++++++ coderd/database/queries.sql.go | 59 ++- coderd/database/queries/insights.sql | 53 +-- 5 files changed, 510 insertions(+), 71 deletions(-) diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 80f0507f59665..a2e613b25d991 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -427,13 +427,15 @@ CREATE FUNCTION record_user_status_change() RETURNS trigger LANGUAGE plpgsql AS $$ BEGIN - IF OLD.status IS DISTINCT FROM NEW.status THEN + IF TG_OP = 'INSERT' OR OLD.status IS DISTINCT FROM NEW.status THEN INSERT INTO user_status_changes ( user_id, - new_status + new_status, + changed_at ) VALUES ( NEW.id, - NEW.status + NEW.status, + NEW.updated_at ); END IF; RETURN NEW; @@ -2268,7 +2270,7 @@ CREATE TRIGGER trigger_upsert_user_links BEFORE INSERT OR UPDATE ON user_links F CREATE TRIGGER update_notification_message_dedupe_hash BEFORE INSERT OR UPDATE ON notification_messages FOR EACH ROW EXECUTE FUNCTION compute_notification_message_dedupe_hash(); -CREATE TRIGGER user_status_change_trigger BEFORE UPDATE ON users FOR EACH ROW EXECUTE FUNCTION record_user_status_change(); +CREATE TRIGGER user_status_change_trigger AFTER INSERT OR UPDATE ON users FOR EACH ROW EXECUTE FUNCTION record_user_status_change(); ALTER TABLE ONLY api_keys ADD CONSTRAINT api_keys_user_id_uuid_fkey FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; diff --git a/coderd/database/migrations/000279_user_status_changes.up.sql b/coderd/database/migrations/000279_user_status_changes.up.sql index 545705aba50eb..f16164862b3e5 100644 --- a/coderd/database/migrations/000279_user_status_changes.up.sql +++ b/coderd/database/migrations/000279_user_status_changes.up.sql @@ -7,7 +7,6 @@ CREATE TABLE user_status_changes ( COMMENT ON TABLE user_status_changes IS 'Tracks the history of user status changes'; -CREATE INDEX idx_user_status_changes_user_id ON user_status_changes(user_id); CREATE INDEX idx_user_status_changes_changed_at ON user_status_changes(changed_at); INSERT INTO user_status_changes ( @@ -22,15 +21,17 @@ SELECT FROM users WHERE NOT deleted; -CREATE FUNCTION record_user_status_change() RETURNS trigger AS $$ +CREATE OR REPLACE FUNCTION record_user_status_change() RETURNS trigger AS $$ BEGIN - IF OLD.status IS DISTINCT FROM NEW.status THEN + IF TG_OP = 'INSERT' OR OLD.status IS DISTINCT FROM NEW.status THEN INSERT INTO user_status_changes ( user_id, - new_status + new_status, + changed_at ) VALUES ( NEW.id, - NEW.status + NEW.status, + NEW.updated_at ); END IF; RETURN NEW; @@ -38,6 +39,6 @@ END; $$ LANGUAGE plpgsql; CREATE TRIGGER user_status_change_trigger - BEFORE UPDATE ON users + AFTER INSERT OR UPDATE ON users FOR EACH ROW EXECUTE FUNCTION record_user_status_change(); diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index 28d7108ae31ad..3da6499f3ec82 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -2255,6 +2255,452 @@ func TestGroupRemovalTrigger(t *testing.T) { }, db2sdk.List(extraUserGroups, onlyGroupIDs)) } +func TestGetUserStatusCountsByDay(t *testing.T) { + t.Parallel() + + t.Run("No Users", func(t *testing.T) { + t.Parallel() + db, _ := dbtestutil.NewDB(t) + ctx := testutil.Context(t, testutil.WaitShort) + + end := dbtime.Now() + start := end.Add(-30 * 24 * time.Hour) + + counts, err := db.GetUserStatusCountsByDay(ctx, database.GetUserStatusCountsByDayParams{ + StartTime: start, + EndTime: end, + }) + require.NoError(t, err) + require.Empty(t, counts, "should return no results when there are no users") + }) + + t.Run("Single User/Single State", func(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + status database.UserStatus + }{ + { + name: "Active Only", + status: database.UserStatusActive, + }, + { + name: "Dormant Only", + status: database.UserStatusDormant, + }, + { + name: "Suspended Only", + status: database.UserStatusSuspended, + }, + // { + // name: "Deleted Only", + // status: database.UserStatusDeleted, + // }, + } + + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + db, _ := dbtestutil.NewDB(t) + ctx := testutil.Context(t, testutil.WaitShort) + + // Create a user that's been in the specified status for the past 30 days + now := dbtime.Now() + createdAt := now.Add(-29 * 24 * time.Hour) + dbgen.User(t, db, database.User{ + Status: tc.status, + CreatedAt: createdAt, + UpdatedAt: createdAt, + }) + + // Query for the last 30 days + counts, err := db.GetUserStatusCountsByDay(ctx, database.GetUserStatusCountsByDayParams{ + StartTime: createdAt, + EndTime: now, + }) + require.NoError(t, err) + require.NotEmpty(t, counts, "should return results") + + // We should have an entry for each day, all with 1 user in the specified status + require.Len(t, counts, 30, "should have 30 days of data") + for _, count := range counts { + if count.Status.Valid && count.Status.UserStatus == tc.status { + require.Equal(t, int64(1), count.Count, "should have 1 %s user", tc.status) + } else { + require.Equal(t, int64(0), count.Count, "should have 0 users for other statuses") + } + } + }) + } + }) + + t.Run("Single Transition", func(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + initialStatus database.UserStatus + targetStatus database.UserStatus + }{ + { + name: "Active to Dormant", + initialStatus: database.UserStatusActive, + targetStatus: database.UserStatusDormant, + }, + { + name: "Active to Suspended", + initialStatus: database.UserStatusActive, + targetStatus: database.UserStatusSuspended, + }, + // { + // name: "Active to Deleted", + // initialStatus: database.UserStatusActive, + // targetStatus: database.UserStatusDeleted, + // }, + { + name: "Dormant to Active", + initialStatus: database.UserStatusDormant, + targetStatus: database.UserStatusActive, + }, + { + name: "Dormant to Suspended", + initialStatus: database.UserStatusDormant, + targetStatus: database.UserStatusSuspended, + }, + // { + // name: "Dormant to Deleted", + // initialStatus: database.UserStatusDormant, + // targetStatus: database.UserStatusDeleted, + // }, + { + name: "Suspended to Active", + initialStatus: database.UserStatusSuspended, + targetStatus: database.UserStatusActive, + }, + { + name: "Suspended to Dormant", + initialStatus: database.UserStatusSuspended, + targetStatus: database.UserStatusDormant, + }, + // { + // name: "Suspended to Deleted", + // initialStatus: database.UserStatusSuspended, + // targetStatus: database.UserStatusDeleted, + // }, + } + + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + db, _ := dbtestutil.NewDB(t) + ctx := testutil.Context(t, testutil.WaitShort) + + // Create a user that starts with initial status + now := dbtime.Now() + createdAt := now.Add(-5 * 24 * time.Hour) // 5 days ago + user := dbgen.User(t, db, database.User{ + Status: tc.initialStatus, + CreatedAt: createdAt, + UpdatedAt: createdAt, + }) + + // After 2 days, change status to target status + statusChangeTime := createdAt.Add(2 * 24 * time.Hour) + user, err := db.UpdateUserStatus(ctx, database.UpdateUserStatusParams{ + ID: user.ID, + Status: tc.targetStatus, + UpdatedAt: statusChangeTime, + }) + require.NoError(t, err) + + // Query for the last 5 days + counts, err := db.GetUserStatusCountsByDay(ctx, database.GetUserStatusCountsByDayParams{ + StartTime: createdAt, + EndTime: now, + }) + require.NoError(t, err) + require.NotEmpty(t, counts, "should return results") + + // We should have entries for each day + require.Len(t, counts, 6, "should have 6 days of data") + + // Helper to get count for a specific day and status + getCount := func(day time.Time, status database.UserStatus) int64 { + day = day.Truncate(24 * time.Hour) + for _, c := range counts { + if c.Day.Truncate(24*time.Hour).Equal(day) && c.Status.Valid && c.Status.UserStatus == status { + return c.Count + } + } + return 0 + } + + // First 2 days should show initial status + require.Equal(t, int64(1), getCount(createdAt, tc.initialStatus), "day 1 should be %s", tc.initialStatus) + require.Equal(t, int64(1), getCount(createdAt.Add(24*time.Hour), tc.initialStatus), "day 2 should be %s", tc.initialStatus) + + // Remaining days should show target status + for i := 2; i < 6; i++ { + dayTime := createdAt.Add(time.Duration(i) * 24 * time.Hour) + require.Equal(t, int64(1), getCount(dayTime, tc.targetStatus), + "day %d should be %s", i+1, tc.targetStatus) + require.Equal(t, int64(0), getCount(dayTime, tc.initialStatus), + "day %d should have no %s users", i+1, tc.initialStatus) + } + }) + } + }) + + t.Run("Two Users Transitioning", func(t *testing.T) { + t.Parallel() + + type transition struct { + from database.UserStatus + to database.UserStatus + } + + type testCase struct { + name string + user1Transition transition + user2Transition transition + expectedCounts map[string]map[database.UserStatus]int64 + } + + testCases := []testCase{ + { + name: "Active->Dormant and Dormant->Suspended", + user1Transition: transition{ + from: database.UserStatusActive, + to: database.UserStatusDormant, + }, + user2Transition: transition{ + from: database.UserStatusDormant, + to: database.UserStatusSuspended, + }, + expectedCounts: map[string]map[database.UserStatus]int64{ + "initial": { + database.UserStatusActive: 1, + database.UserStatusDormant: 1, + database.UserStatusSuspended: 0, + }, + "between": { + database.UserStatusActive: 0, + database.UserStatusDormant: 2, + database.UserStatusSuspended: 0, + }, + "final": { + database.UserStatusActive: 0, + database.UserStatusDormant: 1, + database.UserStatusSuspended: 1, + }, + }, + }, + { + name: "Suspended->Active and Active->Dormant", + user1Transition: transition{ + from: database.UserStatusSuspended, + to: database.UserStatusActive, + }, + user2Transition: transition{ + from: database.UserStatusActive, + to: database.UserStatusDormant, + }, + expectedCounts: map[string]map[database.UserStatus]int64{ + "initial": { + database.UserStatusActive: 1, + database.UserStatusDormant: 0, + database.UserStatusSuspended: 1, + }, + "between": { + database.UserStatusActive: 2, + database.UserStatusDormant: 0, + database.UserStatusSuspended: 0, + }, + "final": { + database.UserStatusActive: 1, + database.UserStatusDormant: 1, + database.UserStatusSuspended: 0, + }, + }, + }, + { + name: "Dormant->Active and Suspended->Dormant", + user1Transition: transition{ + from: database.UserStatusDormant, + to: database.UserStatusActive, + }, + user2Transition: transition{ + from: database.UserStatusSuspended, + to: database.UserStatusDormant, + }, + expectedCounts: map[string]map[database.UserStatus]int64{ + "initial": { + database.UserStatusActive: 0, + database.UserStatusDormant: 1, + database.UserStatusSuspended: 1, + }, + "between": { + database.UserStatusActive: 1, + database.UserStatusDormant: 0, + database.UserStatusSuspended: 1, + }, + "final": { + database.UserStatusActive: 1, + database.UserStatusDormant: 1, + database.UserStatusSuspended: 0, + }, + }, + }, + { + name: "Active->Suspended and Suspended->Active", + user1Transition: transition{ + from: database.UserStatusActive, + to: database.UserStatusSuspended, + }, + user2Transition: transition{ + from: database.UserStatusSuspended, + to: database.UserStatusActive, + }, + expectedCounts: map[string]map[database.UserStatus]int64{ + "initial": { + database.UserStatusActive: 1, + database.UserStatusDormant: 0, + database.UserStatusSuspended: 1, + }, + "between": { + database.UserStatusActive: 0, + database.UserStatusDormant: 0, + database.UserStatusSuspended: 2, + }, + "final": { + database.UserStatusActive: 1, + database.UserStatusDormant: 0, + database.UserStatusSuspended: 1, + }, + }, + }, + { + name: "Dormant->Suspended and Dormant->Active", + user1Transition: transition{ + from: database.UserStatusDormant, + to: database.UserStatusSuspended, + }, + user2Transition: transition{ + from: database.UserStatusDormant, + to: database.UserStatusActive, + }, + expectedCounts: map[string]map[database.UserStatus]int64{ + "initial": { + database.UserStatusActive: 0, + database.UserStatusDormant: 2, + database.UserStatusSuspended: 0, + }, + "between": { + database.UserStatusActive: 0, + database.UserStatusDormant: 1, + database.UserStatusSuspended: 1, + }, + "final": { + database.UserStatusActive: 1, + database.UserStatusDormant: 0, + database.UserStatusSuspended: 1, + }, + }, + }, + } + + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + db, _ := dbtestutil.NewDB(t) + ctx := testutil.Context(t, testutil.WaitShort) + + now := dbtime.Now() + createdAt := now.Add(-5 * 24 * time.Hour) // 5 days ago + + user1 := dbgen.User(t, db, database.User{ + Status: tc.user1Transition.from, + CreatedAt: createdAt, + UpdatedAt: createdAt, + }) + user2 := dbgen.User(t, db, database.User{ + Status: tc.user2Transition.from, + CreatedAt: createdAt, + UpdatedAt: createdAt, + }) + + // First transition at 2 days + user1TransitionTime := createdAt.Add(2 * 24 * time.Hour) + user1, err := db.UpdateUserStatus(ctx, database.UpdateUserStatusParams{ + ID: user1.ID, + Status: tc.user1Transition.to, + UpdatedAt: user1TransitionTime, + }) + require.NoError(t, err) + + // Second transition at 4 days + user2TransitionTime := createdAt.Add(4 * 24 * time.Hour) + user2, err = db.UpdateUserStatus(ctx, database.UpdateUserStatusParams{ + ID: user2.ID, + Status: tc.user2Transition.to, + UpdatedAt: user2TransitionTime, + }) + require.NoError(t, err) + + // Query for the last 5 days + counts, err := db.GetUserStatusCountsByDay(ctx, database.GetUserStatusCountsByDayParams{ + StartTime: createdAt, + EndTime: now, + }) + require.NoError(t, err) + require.NotEmpty(t, counts) + + // Helper to get count for a specific day and status + getCount := func(day time.Time, status database.UserStatus) int64 { + day = day.Truncate(24 * time.Hour) + for _, c := range counts { + if c.Day.Truncate(24*time.Hour).Equal(day) && c.Status.Valid && c.Status.UserStatus == status { + return c.Count + } + } + return 0 + } + + // Check initial period (days 0-1) + for i := 0; i < 2; i++ { + dayTime := createdAt.Add(time.Duration(i) * 24 * time.Hour) + for status, expectedCount := range tc.expectedCounts["initial"] { + require.Equal(t, expectedCount, getCount(dayTime, status), + "day %d should have %d %s users", i+1, expectedCount, status) + } + } + + // Check between transitions (days 2-3) + for i := 2; i < 4; i++ { + dayTime := createdAt.Add(time.Duration(i) * 24 * time.Hour) + for status, expectedCount := range tc.expectedCounts["between"] { + require.Equal(t, expectedCount, getCount(dayTime, status), + "day %d should have %d %s users", i+1, expectedCount, status) + } + } + + // Check final period (days 4-5) + for i := 4; i < 6; i++ { + dayTime := createdAt.Add(time.Duration(i) * 24 * time.Hour) + for status, expectedCount := range tc.expectedCounts["final"] { + require.Equal(t, expectedCount, getCount(dayTime, status), + "day %d should have %d %s users", i+1, expectedCount, status) + } + } + }) + } + }) +} + func requireUsersMatch(t testing.TB, expected []database.User, found []database.GetUsersRow, msg string) { t.Helper() require.ElementsMatch(t, expected, database.ConvertUserRows(found), msg) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 3c902fe22f532..e08d1e378ce35 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3106,46 +3106,41 @@ WITH dates AS ( '1 day'::interval ) AS day ), -initial_statuses AS ( - -- Get the status of each user right before the start date - SELECT DISTINCT ON (user_id) - user_id, - new_status as status - FROM - user_status_changes - WHERE - changed_at < $1::timestamptz - ORDER BY - user_id, - changed_at DESC +all_users AS ( + -- Get all users who have had any status changes in or before the range + SELECT DISTINCT user_id + FROM user_status_changes + WHERE changed_at <= $2::timestamptz ), -relevant_changes AS ( - -- Get only the status changes within our date range - SELECT - date_trunc('day', changed_at)::date AS day, +initial_statuses AS ( + -- Get the status of each user right before each day + SELECT DISTINCT ON (user_id, day) user_id, + day, new_status as status FROM - user_status_changes - WHERE - changed_at >= $1::timestamptz - AND changed_at <= $2::timestamptz + all_users + CROSS JOIN + dates + LEFT JOIN LATERAL ( + SELECT new_status, changed_at + FROM user_status_changes + WHERE user_status_changes.user_id = all_users.user_id + AND changed_at < day + interval '1 day' + ORDER BY changed_at DESC + LIMIT 1 + ) changes ON true + WHERE changes.new_status IS NOT NULL ), daily_status AS ( - -- Combine initial statuses with changes SELECT d.day, - COALESCE(rc.status, i.status) as status, - COALESCE(rc.user_id, i.user_id) as user_id + i.status, + i.user_id FROM dates d - CROSS JOIN - initial_statuses i LEFT JOIN - relevant_changes rc - ON - rc.day = d.day - AND rc.user_id = i.user_id + initial_statuses i ON i.day = d.day ) SELECT day, @@ -3169,9 +3164,9 @@ type GetUserStatusCountsByDayParams struct { } type GetUserStatusCountsByDayRow struct { - Day time.Time `db:"day" json:"day"` - Status UserStatus `db:"status" json:"status"` - Count int64 `db:"count" json:"count"` + Day time.Time `db:"day" json:"day"` + Status NullUserStatus `db:"status" json:"status"` + Count int64 `db:"count" json:"count"` } func (q *sqlQuerier) GetUserStatusCountsByDay(ctx context.Context, arg GetUserStatusCountsByDayParams) ([]GetUserStatusCountsByDayRow, error) { diff --git a/coderd/database/queries/insights.sql b/coderd/database/queries/insights.sql index 248edd89fac23..1edb8666e8f1e 100644 --- a/coderd/database/queries/insights.sql +++ b/coderd/database/queries/insights.sql @@ -784,46 +784,41 @@ WITH dates AS ( '1 day'::interval ) AS day ), -initial_statuses AS ( - -- Get the status of each user right before the start date - SELECT DISTINCT ON (user_id) - user_id, - new_status as status - FROM - user_status_changes - WHERE - changed_at < @start_time::timestamptz - ORDER BY - user_id, - changed_at DESC +all_users AS ( + -- Get all users who have had any status changes in or before the range + SELECT DISTINCT user_id + FROM user_status_changes + WHERE changed_at <= @end_time::timestamptz ), -relevant_changes AS ( - -- Get only the status changes within our date range - SELECT - date_trunc('day', changed_at)::date AS day, +initial_statuses AS ( + -- Get the status of each user right before each day + SELECT DISTINCT ON (user_id, day) user_id, + day, new_status as status FROM - user_status_changes - WHERE - changed_at >= @start_time::timestamptz - AND changed_at <= @end_time::timestamptz + all_users + CROSS JOIN + dates + LEFT JOIN LATERAL ( + SELECT new_status, changed_at + FROM user_status_changes + WHERE user_status_changes.user_id = all_users.user_id + AND changed_at < day + interval '1 day' + ORDER BY changed_at DESC + LIMIT 1 + ) changes ON true + WHERE changes.new_status IS NOT NULL ), daily_status AS ( - -- Combine initial statuses with changes SELECT d.day, - COALESCE(rc.status, i.status) as status, - COALESCE(rc.user_id, i.user_id) as user_id + i.status, + i.user_id FROM dates d - CROSS JOIN - initial_statuses i LEFT JOIN - relevant_changes rc - ON - rc.day = d.day - AND rc.user_id = i.user_id + initial_statuses i ON i.day = d.day ) SELECT day, From 7b2c25902b51ed0266f6d71f32d5e04433637e5e Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Tue, 17 Dec 2024 08:14:35 +0000 Subject: [PATCH 05/32] make gen --- coderd/database/dump.sql | 2 -- coderd/database/migrations/000279_user_status_changes.down.sql | 1 - 2 files changed, 3 deletions(-) diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index a2e613b25d991..a03bc5dedcd0b 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -2126,8 +2126,6 @@ CREATE INDEX idx_tailnet_tunnels_src_id ON tailnet_tunnels USING hash (src_id); CREATE INDEX idx_user_status_changes_changed_at ON user_status_changes USING btree (changed_at); -CREATE INDEX idx_user_status_changes_user_id ON user_status_changes USING btree (user_id); - CREATE UNIQUE INDEX idx_users_email ON users USING btree (email) WHERE (deleted = false); CREATE UNIQUE INDEX idx_users_username ON users USING btree (username) WHERE (deleted = false); diff --git a/coderd/database/migrations/000279_user_status_changes.down.sql b/coderd/database/migrations/000279_user_status_changes.down.sql index b75acbcafca46..30514ef467ebe 100644 --- a/coderd/database/migrations/000279_user_status_changes.down.sql +++ b/coderd/database/migrations/000279_user_status_changes.down.sql @@ -6,7 +6,6 @@ DROP FUNCTION IF EXISTS record_user_status_change(); -- Drop the indexes DROP INDEX IF EXISTS idx_user_status_changes_changed_at; -DROP INDEX IF EXISTS idx_user_status_changes_user_id; -- Drop the table DROP TABLE IF EXISTS user_status_changes; From 89177b2d376c24f54fba8f1b8c451b80a988b595 Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Tue, 17 Dec 2024 11:37:35 +0000 Subject: [PATCH 06/32] fix dbauthz tests --- coderd/database/dbauthz/dbauthz.go | 2 +- coderd/database/dbauthz/dbauthz_test.go | 6 +++++ coderd/database/dbmem/dbmem.go | 36 ++++++++++++++++++++++++- 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 29b3f9f5105e1..df0b4ec52167b 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -2422,7 +2422,7 @@ func (q *querier) GetUserNotificationPreferences(ctx context.Context, userID uui } func (q *querier) GetUserStatusCountsByDay(ctx context.Context, arg database.GetUserStatusCountsByDayParams) ([]database.GetUserStatusCountsByDayRow, error) { - if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil { + if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceUser); err != nil { return nil, err } return q.db.GetUserStatusCountsByDay(ctx, arg) diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 93e9a4318d1ed..2b74538fa28b5 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -1708,6 +1708,12 @@ func (s *MethodTestSuite) TestUser() { rbac.ResourceTemplate.InOrg(orgID), policy.ActionRead, ) })) + s.Run("GetUserStatusCountsByDay", s.Subtest(func(db database.Store, check *expects) { + check.Args(database.GetUserStatusCountsByDayParams{ + StartTime: time.Now().Add(-time.Hour * 24 * 30), + EndTime: time.Now(), + }).Asserts(rbac.ResourceUser, policy.ActionRead) + })) } func (s *MethodTestSuite) TestWorkspace() { diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index a4244544ee5c8..03603504bd56f 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -88,6 +88,7 @@ func New() database.Store { customRoles: make([]database.CustomRole, 0), locks: map[int64]struct{}{}, runtimeConfig: map[string]string{}, + userStatusChanges: make([]database.UserStatusChange, 0), }, } // Always start with a default org. Matching migration 198. @@ -256,6 +257,7 @@ type data struct { lastLicenseID int32 defaultProxyDisplayName string defaultProxyIconURL string + userStatusChanges []database.UserStatusChange } func tryPercentile(fs []float64, p float64) float64 { @@ -5678,7 +5680,21 @@ func (q *FakeQuerier) GetUserStatusCountsByDay(_ context.Context, arg database.G return nil, err } - panic("not implemented") + result := make([]database.GetUserStatusCountsByDayRow, 0) + for _, change := range q.userStatusChanges { + if change.ChangedAt.Before(arg.StartTime) || change.ChangedAt.After(arg.EndTime) { + continue + } + result = append(result, database.GetUserStatusCountsByDayRow{ + Status: database.NullUserStatus{ + UserStatus: change.NewStatus, + Valid: true, + }, + Count: 1, + }) + } + + return result, nil } func (q *FakeQuerier) GetUserWorkspaceBuildParameters(_ context.Context, params database.GetUserWorkspaceBuildParametersParams) ([]database.GetUserWorkspaceBuildParametersRow, error) { @@ -8033,6 +8049,12 @@ func (q *FakeQuerier) InsertUser(_ context.Context, arg database.InsertUserParam sort.Slice(q.users, func(i, j int) bool { return q.users[i].CreatedAt.Before(q.users[j].CreatedAt) }) + + q.userStatusChanges = append(q.userStatusChanges, database.UserStatusChange{ + UserID: user.ID, + NewStatus: user.Status, + ChangedAt: user.UpdatedAt, + }) return user, nil } @@ -9074,12 +9096,18 @@ func (q *FakeQuerier) UpdateInactiveUsersToDormant(_ context.Context, params dat Username: user.Username, LastSeenAt: user.LastSeenAt, }) + q.userStatusChanges = append(q.userStatusChanges, database.UserStatusChange{ + UserID: user.ID, + NewStatus: database.UserStatusDormant, + ChangedAt: params.UpdatedAt, + }) } } if len(updated) == 0 { return nil, sql.ErrNoRows } + return updated, nil } @@ -9880,6 +9908,12 @@ func (q *FakeQuerier) UpdateUserStatus(_ context.Context, arg database.UpdateUse user.Status = arg.Status user.UpdatedAt = arg.UpdatedAt q.users[index] = user + + q.userStatusChanges = append(q.userStatusChanges, database.UserStatusChange{ + UserID: user.ID, + NewStatus: user.Status, + ChangedAt: user.UpdatedAt, + }) return user, nil } return database.User{}, sql.ErrNoRows From 877517fe2ae188527474b6d768d3eee18c67ef35 Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Mon, 23 Dec 2024 08:57:32 +0000 Subject: [PATCH 07/32] do the plumbing to get sql, api and frontend talking to one another --- coderd/apidoc/docs.go | 61 ++++++ coderd/apidoc/swagger.json | 57 ++++++ coderd/coderd.go | 1 + coderd/database/dbauthz/dbauthz.go | 4 +- coderd/database/dbauthz/dbauthz_test.go | 4 +- coderd/database/dbmem/dbmem.go | 12 +- coderd/database/dbmetrics/querymetrics.go | 6 +- coderd/database/dbmock/dbmock.go | 14 +- coderd/database/querier.go | 2 +- coderd/database/querier_test.go | 109 +++-------- coderd/database/queries.sql.go | 104 +++-------- coderd/database/queries/insights.sql | 67 +------ coderd/insights.go | 67 +++++++ codersdk/insights.go | 31 ++++ docs/reference/api/insights.md | 50 +++++ docs/reference/api/schemas.md | 44 +++++ site/src/api/api.ts | 13 ++ site/src/api/queries/insights.ts | 7 + site/src/api/queries/users.ts | 1 - site/src/api/typesGenerated.ts | 16 ++ .../ActiveUserChart.stories.tsx | 69 ++++++- .../ActiveUserChart/ActiveUserChart.tsx | 121 +++++++++--- .../GeneralSettingsPage.tsx | 7 +- .../GeneralSettingsPageView.stories.tsx | 174 +++++++++++------- .../GeneralSettingsPageView.tsx | 82 ++++----- .../TemplateInsightsPage.tsx | 12 +- 26 files changed, 740 insertions(+), 395 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index a8bfcb2af3b19..983aa7227a4a5 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -1398,6 +1398,40 @@ const docTemplate = `{ } } }, + "/insights/user-status-counts-over-time": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": [ + "application/json" + ], + "tags": [ + "Insights" + ], + "summary": "Get insights about user status counts over time", + "operationId": "get-insights-about-user-status-counts-over-time", + "parameters": [ + { + "type": "integer", + "description": "Time-zone offset (e.g. -2)", + "name": "tz_offset", + "in": "query", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/codersdk.GetUserStatusChangesResponse" + } + } + } + } + }, "/integrations/jfrog/xray-scan": { "get": { "security": [ @@ -11207,6 +11241,20 @@ const docTemplate = `{ } } }, + "codersdk.GetUserStatusChangesResponse": { + "type": "object", + "properties": { + "status_counts": { + "type": "object", + "additionalProperties": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.UserStatusChangeCount" + } + } + } + } + }, "codersdk.GetUsersResponse": { "type": "object", "properties": { @@ -14570,6 +14618,19 @@ const docTemplate = `{ "UserStatusSuspended" ] }, + "codersdk.UserStatusChangeCount": { + "type": "object", + "properties": { + "count": { + "type": "integer", + "example": 10 + }, + "date": { + "type": "string", + "format": "date-time" + } + } + }, "codersdk.ValidateUserPasswordRequest": { "type": "object", "required": [ diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index d7c32d8a33a52..d700a3089114a 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -1219,6 +1219,36 @@ } } }, + "/insights/user-status-counts-over-time": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": ["application/json"], + "tags": ["Insights"], + "summary": "Get insights about user status counts over time", + "operationId": "get-insights-about-user-status-counts-over-time", + "parameters": [ + { + "type": "integer", + "description": "Time-zone offset (e.g. -2)", + "name": "tz_offset", + "in": "query", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/codersdk.GetUserStatusChangesResponse" + } + } + } + } + }, "/integrations/jfrog/xray-scan": { "get": { "security": [ @@ -10054,6 +10084,20 @@ } } }, + "codersdk.GetUserStatusChangesResponse": { + "type": "object", + "properties": { + "status_counts": { + "type": "object", + "additionalProperties": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.UserStatusChangeCount" + } + } + } + } + }, "codersdk.GetUsersResponse": { "type": "object", "properties": { @@ -13244,6 +13288,19 @@ "UserStatusSuspended" ] }, + "codersdk.UserStatusChangeCount": { + "type": "object", + "properties": { + "count": { + "type": "integer", + "example": 10 + }, + "date": { + "type": "string", + "format": "date-time" + } + } + }, "codersdk.ValidateUserPasswordRequest": { "type": "object", "required": ["password"], diff --git a/coderd/coderd.go b/coderd/coderd.go index fd8a10a44f140..c197c08fd5cd9 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -1281,6 +1281,7 @@ func New(options *Options) *API { r.Use(apiKeyMiddleware) r.Get("/daus", api.deploymentDAUs) r.Get("/user-activity", api.insightsUserActivity) + r.Get("/user-status-counts-over-time", api.insightsUserStatusCountsOverTime) r.Get("/user-latency", api.insightsUserLatency) r.Get("/templates", api.insightsTemplates) }) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index df0b4ec52167b..f0201459c27c8 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -2421,11 +2421,11 @@ func (q *querier) GetUserNotificationPreferences(ctx context.Context, userID uui return q.db.GetUserNotificationPreferences(ctx, userID) } -func (q *querier) GetUserStatusCountsByDay(ctx context.Context, arg database.GetUserStatusCountsByDayParams) ([]database.GetUserStatusCountsByDayRow, error) { +func (q *querier) GetUserStatusChanges(ctx context.Context, arg database.GetUserStatusChangesParams) ([]database.UserStatusChange, error) { if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceUser); err != nil { return nil, err } - return q.db.GetUserStatusCountsByDay(ctx, arg) + return q.db.GetUserStatusChanges(ctx, arg) } func (q *querier) GetUserWorkspaceBuildParameters(ctx context.Context, params database.GetUserWorkspaceBuildParametersParams) ([]database.GetUserWorkspaceBuildParametersRow, error) { diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 2b74538fa28b5..b1c5898a82e34 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -1708,8 +1708,8 @@ func (s *MethodTestSuite) TestUser() { rbac.ResourceTemplate.InOrg(orgID), policy.ActionRead, ) })) - s.Run("GetUserStatusCountsByDay", s.Subtest(func(db database.Store, check *expects) { - check.Args(database.GetUserStatusCountsByDayParams{ + s.Run("GetUserStatusChanges", s.Subtest(func(db database.Store, check *expects) { + check.Args(database.GetUserStatusChangesParams{ StartTime: time.Now().Add(-time.Hour * 24 * 30), EndTime: time.Now(), }).Asserts(rbac.ResourceUser, policy.ActionRead) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 03603504bd56f..d236ec69a2e32 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -5671,7 +5671,7 @@ func (q *FakeQuerier) GetUserNotificationPreferences(_ context.Context, userID u return out, nil } -func (q *FakeQuerier) GetUserStatusCountsByDay(_ context.Context, arg database.GetUserStatusCountsByDayParams) ([]database.GetUserStatusCountsByDayRow, error) { +func (q *FakeQuerier) GetUserStatusChanges(_ context.Context, arg database.GetUserStatusChangesParams) ([]database.UserStatusChange, error) { q.mutex.RLock() defer q.mutex.RUnlock() @@ -5680,18 +5680,12 @@ func (q *FakeQuerier) GetUserStatusCountsByDay(_ context.Context, arg database.G return nil, err } - result := make([]database.GetUserStatusCountsByDayRow, 0) + result := make([]database.UserStatusChange, 0) for _, change := range q.userStatusChanges { if change.ChangedAt.Before(arg.StartTime) || change.ChangedAt.After(arg.EndTime) { continue } - result = append(result, database.GetUserStatusCountsByDayRow{ - Status: database.NullUserStatus{ - UserStatus: change.NewStatus, - Valid: true, - }, - Count: 1, - }) + result = append(result, change) } return result, nil diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index 595df4464e691..7c001489e3a7e 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -1344,10 +1344,10 @@ func (m queryMetricsStore) GetUserNotificationPreferences(ctx context.Context, u return r0, r1 } -func (m queryMetricsStore) GetUserStatusCountsByDay(ctx context.Context, arg database.GetUserStatusCountsByDayParams) ([]database.GetUserStatusCountsByDayRow, error) { +func (m queryMetricsStore) GetUserStatusChanges(ctx context.Context, arg database.GetUserStatusChangesParams) ([]database.UserStatusChange, error) { start := time.Now() - r0, r1 := m.s.GetUserStatusCountsByDay(ctx, arg) - m.queryLatencies.WithLabelValues("GetUserStatusCountsByDay").Observe(time.Since(start).Seconds()) + r0, r1 := m.s.GetUserStatusChanges(ctx, arg) + m.queryLatencies.WithLabelValues("GetUserStatusChanges").Observe(time.Since(start).Seconds()) return r0, r1 } diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index aabc4f6828302..6391f4a2277a1 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -2825,19 +2825,19 @@ func (mr *MockStoreMockRecorder) GetUserNotificationPreferences(arg0, arg1 any) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserNotificationPreferences", reflect.TypeOf((*MockStore)(nil).GetUserNotificationPreferences), arg0, arg1) } -// GetUserStatusCountsByDay mocks base method. -func (m *MockStore) GetUserStatusCountsByDay(arg0 context.Context, arg1 database.GetUserStatusCountsByDayParams) ([]database.GetUserStatusCountsByDayRow, error) { +// GetUserStatusChanges mocks base method. +func (m *MockStore) GetUserStatusChanges(arg0 context.Context, arg1 database.GetUserStatusChangesParams) ([]database.UserStatusChange, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetUserStatusCountsByDay", arg0, arg1) - ret0, _ := ret[0].([]database.GetUserStatusCountsByDayRow) + ret := m.ctrl.Call(m, "GetUserStatusChanges", arg0, arg1) + ret0, _ := ret[0].([]database.UserStatusChange) ret1, _ := ret[1].(error) return ret0, ret1 } -// GetUserStatusCountsByDay indicates an expected call of GetUserStatusCountsByDay. -func (mr *MockStoreMockRecorder) GetUserStatusCountsByDay(arg0, arg1 any) *gomock.Call { +// GetUserStatusChanges indicates an expected call of GetUserStatusChanges. +func (mr *MockStoreMockRecorder) GetUserStatusChanges(arg0, arg1 any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserStatusCountsByDay", reflect.TypeOf((*MockStore)(nil).GetUserStatusCountsByDay), arg0, arg1) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserStatusChanges", reflect.TypeOf((*MockStore)(nil).GetUserStatusChanges), arg0, arg1) } // GetUserWorkspaceBuildParameters mocks base method. diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 36401598c8d40..332a2c2c535bb 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -289,7 +289,7 @@ type sqlcQuerier interface { GetUserLinkByUserIDLoginType(ctx context.Context, arg GetUserLinkByUserIDLoginTypeParams) (UserLink, error) GetUserLinksByUserID(ctx context.Context, userID uuid.UUID) ([]UserLink, error) GetUserNotificationPreferences(ctx context.Context, userID uuid.UUID) ([]NotificationPreference, error) - GetUserStatusCountsByDay(ctx context.Context, arg GetUserStatusCountsByDayParams) ([]GetUserStatusCountsByDayRow, error) + GetUserStatusChanges(ctx context.Context, arg GetUserStatusChangesParams) ([]UserStatusChange, error) GetUserWorkspaceBuildParameters(ctx context.Context, arg GetUserWorkspaceBuildParametersParams) ([]GetUserWorkspaceBuildParametersRow, error) // This will never return deleted users. GetUsers(ctx context.Context, arg GetUsersParams) ([]GetUsersRow, error) diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index 3da6499f3ec82..c724789df58ac 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -2255,7 +2255,7 @@ func TestGroupRemovalTrigger(t *testing.T) { }, db2sdk.List(extraUserGroups, onlyGroupIDs)) } -func TestGetUserStatusCountsByDay(t *testing.T) { +func TestGetUserStatusChanges(t *testing.T) { t.Parallel() t.Run("No Users", func(t *testing.T) { @@ -2266,7 +2266,7 @@ func TestGetUserStatusCountsByDay(t *testing.T) { end := dbtime.Now() start := end.Add(-30 * 24 * time.Hour) - counts, err := db.GetUserStatusCountsByDay(ctx, database.GetUserStatusCountsByDayParams{ + counts, err := db.GetUserStatusChanges(ctx, database.GetUserStatusChangesParams{ StartTime: start, EndTime: end, }) @@ -2316,22 +2316,16 @@ func TestGetUserStatusCountsByDay(t *testing.T) { }) // Query for the last 30 days - counts, err := db.GetUserStatusCountsByDay(ctx, database.GetUserStatusCountsByDayParams{ + userStatusChanges, err := db.GetUserStatusChanges(ctx, database.GetUserStatusChangesParams{ StartTime: createdAt, EndTime: now, }) require.NoError(t, err) - require.NotEmpty(t, counts, "should return results") - - // We should have an entry for each day, all with 1 user in the specified status - require.Len(t, counts, 30, "should have 30 days of data") - for _, count := range counts { - if count.Status.Valid && count.Status.UserStatus == tc.status { - require.Equal(t, int64(1), count.Count, "should have 1 %s user", tc.status) - } else { - require.Equal(t, int64(0), count.Count, "should have 0 users for other statuses") - } - } + require.NotEmpty(t, userStatusChanges, "should return results") + + // We should have an entry for each status change + require.Len(t, userStatusChanges, 1, "should have 1 status change") + require.Equal(t, userStatusChanges[0].NewStatus, tc.status, "should have the correct status") }) } }) @@ -2417,39 +2411,17 @@ func TestGetUserStatusCountsByDay(t *testing.T) { require.NoError(t, err) // Query for the last 5 days - counts, err := db.GetUserStatusCountsByDay(ctx, database.GetUserStatusCountsByDayParams{ + userStatusChanges, err := db.GetUserStatusChanges(ctx, database.GetUserStatusChangesParams{ StartTime: createdAt, EndTime: now, }) require.NoError(t, err) - require.NotEmpty(t, counts, "should return results") - - // We should have entries for each day - require.Len(t, counts, 6, "should have 6 days of data") - - // Helper to get count for a specific day and status - getCount := func(day time.Time, status database.UserStatus) int64 { - day = day.Truncate(24 * time.Hour) - for _, c := range counts { - if c.Day.Truncate(24*time.Hour).Equal(day) && c.Status.Valid && c.Status.UserStatus == status { - return c.Count - } - } - return 0 - } + require.NotEmpty(t, userStatusChanges, "should return results") - // First 2 days should show initial status - require.Equal(t, int64(1), getCount(createdAt, tc.initialStatus), "day 1 should be %s", tc.initialStatus) - require.Equal(t, int64(1), getCount(createdAt.Add(24*time.Hour), tc.initialStatus), "day 2 should be %s", tc.initialStatus) - - // Remaining days should show target status - for i := 2; i < 6; i++ { - dayTime := createdAt.Add(time.Duration(i) * 24 * time.Hour) - require.Equal(t, int64(1), getCount(dayTime, tc.targetStatus), - "day %d should be %s", i+1, tc.targetStatus) - require.Equal(t, int64(0), getCount(dayTime, tc.initialStatus), - "day %d should have no %s users", i+1, tc.initialStatus) - } + // We should have an entry for each status change, including the initial status + require.Len(t, userStatusChanges, 2, "should have 2 status changes") + require.Equal(t, userStatusChanges[0].NewStatus, tc.initialStatus, "should have the initial status") + require.Equal(t, userStatusChanges[1].NewStatus, tc.targetStatus, "should have the target status") }) } }) @@ -2652,50 +2624,23 @@ func TestGetUserStatusCountsByDay(t *testing.T) { require.NoError(t, err) // Query for the last 5 days - counts, err := db.GetUserStatusCountsByDay(ctx, database.GetUserStatusCountsByDayParams{ + userStatusChanges, err := db.GetUserStatusChanges(ctx, database.GetUserStatusChangesParams{ StartTime: createdAt, EndTime: now, }) require.NoError(t, err) - require.NotEmpty(t, counts) - - // Helper to get count for a specific day and status - getCount := func(day time.Time, status database.UserStatus) int64 { - day = day.Truncate(24 * time.Hour) - for _, c := range counts { - if c.Day.Truncate(24*time.Hour).Equal(day) && c.Status.Valid && c.Status.UserStatus == status { - return c.Count - } - } - return 0 - } - - // Check initial period (days 0-1) - for i := 0; i < 2; i++ { - dayTime := createdAt.Add(time.Duration(i) * 24 * time.Hour) - for status, expectedCount := range tc.expectedCounts["initial"] { - require.Equal(t, expectedCount, getCount(dayTime, status), - "day %d should have %d %s users", i+1, expectedCount, status) - } - } - - // Check between transitions (days 2-3) - for i := 2; i < 4; i++ { - dayTime := createdAt.Add(time.Duration(i) * 24 * time.Hour) - for status, expectedCount := range tc.expectedCounts["between"] { - require.Equal(t, expectedCount, getCount(dayTime, status), - "day %d should have %d %s users", i+1, expectedCount, status) - } - } - - // Check final period (days 4-5) - for i := 4; i < 6; i++ { - dayTime := createdAt.Add(time.Duration(i) * 24 * time.Hour) - for status, expectedCount := range tc.expectedCounts["final"] { - require.Equal(t, expectedCount, getCount(dayTime, status), - "day %d should have %d %s users", i+1, expectedCount, status) - } - } + require.NotEmpty(t, userStatusChanges) + + // We should have an entry with the correct status changes for each user, including the initial status + require.Len(t, userStatusChanges, 4, "should have 4 status changes") + require.Equal(t, userStatusChanges[0].UserID, user1.ID, "should have the first user") + require.Equal(t, userStatusChanges[0].NewStatus, tc.user1Transition.from, "should have the first user's initial status") + require.Equal(t, userStatusChanges[1].UserID, user1.ID, "should have the first user") + require.Equal(t, userStatusChanges[1].NewStatus, tc.user1Transition.to, "should have the first user's target status") + require.Equal(t, userStatusChanges[2].UserID, user2.ID, "should have the second user") + require.Equal(t, userStatusChanges[2].NewStatus, tc.user2Transition.from, "should have the second user's initial status") + require.Equal(t, userStatusChanges[3].UserID, user2.ID, "should have the second user") + require.Equal(t, userStatusChanges[3].NewStatus, tc.user2Transition.to, "should have the second user's target status") }) } }) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index e08d1e378ce35..85559b07eebd1 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3094,91 +3094,35 @@ func (q *sqlQuerier) GetUserLatencyInsights(ctx context.Context, arg GetUserLate return items, nil } -const getUserStatusCountsByDay = `-- name: GetUserStatusCountsByDay :many -WITH dates AS ( - -- Generate a series of dates between start and end - SELECT - day::date - FROM - generate_series( - date_trunc('day', $1::timestamptz), - date_trunc('day', $2::timestamptz), - '1 day'::interval - ) AS day -), -all_users AS ( - -- Get all users who have had any status changes in or before the range - SELECT DISTINCT user_id - FROM user_status_changes - WHERE changed_at <= $2::timestamptz -), -initial_statuses AS ( - -- Get the status of each user right before each day - SELECT DISTINCT ON (user_id, day) - user_id, - day, - new_status as status - FROM - all_users - CROSS JOIN - dates - LEFT JOIN LATERAL ( - SELECT new_status, changed_at - FROM user_status_changes - WHERE user_status_changes.user_id = all_users.user_id - AND changed_at < day + interval '1 day' - ORDER BY changed_at DESC - LIMIT 1 - ) changes ON true - WHERE changes.new_status IS NOT NULL -), -daily_status AS ( - SELECT - d.day, - i.status, - i.user_id - FROM - dates d - LEFT JOIN - initial_statuses i ON i.day = d.day -) +const GetUserStatusChanges = `-- name: GetUserStatusChanges :many SELECT - day, - status, - COUNT(*) AS count -FROM - daily_status -WHERE - status IS NOT NULL -GROUP BY - day, - status -ORDER BY - day ASC, - status ASC + id, user_id, new_status, changed_at +FROM user_status_changes +WHERE changed_at >= $1::timestamptz + AND changed_at < $2::timestamptz +ORDER BY changed_at ` -type GetUserStatusCountsByDayParams struct { +type GetUserStatusChangesParams struct { StartTime time.Time `db:"start_time" json:"start_time"` EndTime time.Time `db:"end_time" json:"end_time"` } -type GetUserStatusCountsByDayRow struct { - Day time.Time `db:"day" json:"day"` - Status NullUserStatus `db:"status" json:"status"` - Count int64 `db:"count" json:"count"` -} - -func (q *sqlQuerier) GetUserStatusCountsByDay(ctx context.Context, arg GetUserStatusCountsByDayParams) ([]GetUserStatusCountsByDayRow, error) { - rows, err := q.db.QueryContext(ctx, getUserStatusCountsByDay, arg.StartTime, arg.EndTime) +func (q *sqlQuerier) GetUserStatusChanges(ctx context.Context, arg GetUserStatusChangesParams) ([]UserStatusChange, error) { + rows, err := q.db.QueryContext(ctx, GetUserStatusChanges, arg.StartTime, arg.EndTime) if err != nil { return nil, err } defer rows.Close() - var items []GetUserStatusCountsByDayRow + var items []UserStatusChange for rows.Next() { - var i GetUserStatusCountsByDayRow - if err := rows.Scan(&i.Day, &i.Status, &i.Count); err != nil { + var i UserStatusChange + if err := rows.Scan( + &i.ID, + &i.UserID, + &i.NewStatus, + &i.ChangedAt, + ); err != nil { return nil, err } items = append(items, i) @@ -3488,7 +3432,7 @@ func (q *sqlQuerier) GetJFrogXrayScanByWorkspaceAndAgentID(ctx context.Context, } const upsertJFrogXrayScanByWorkspaceAndAgentID = `-- name: UpsertJFrogXrayScanByWorkspaceAndAgentID :exec -INSERT INTO +INSERT INTO jfrog_xray_scans ( agent_id, workspace_id, @@ -3497,7 +3441,7 @@ INSERT INTO medium, results_url ) -VALUES +VALUES ($1, $2, $3, $4, $5, $6) ON CONFLICT (agent_id, workspace_id) DO UPDATE SET critical = $3, high = $4, medium = $5, results_url = $6 @@ -6372,7 +6316,7 @@ FROM provisioner_keys WHERE organization_id = $1 -AND +AND lower(name) = lower($2) ` @@ -6488,10 +6432,10 @@ WHERE AND -- exclude reserved built-in key id != '00000000-0000-0000-0000-000000000001'::uuid -AND +AND -- exclude reserved user-auth key id != '00000000-0000-0000-0000-000000000002'::uuid -AND +AND -- exclude reserved psk key id != '00000000-0000-0000-0000-000000000003'::uuid ` @@ -8177,7 +8121,7 @@ func (q *sqlQuerier) GetTailnetTunnelPeerIDs(ctx context.Context, srcID uuid.UUI } const updateTailnetPeerStatusByCoordinator = `-- name: UpdateTailnetPeerStatusByCoordinator :exec -UPDATE +UPDATE tailnet_peers SET status = $2 @@ -12790,7 +12734,7 @@ WITH agent_stats AS ( coalesce((PERCENTILE_CONT(0.95) WITHIN GROUP (ORDER BY connection_median_latency_ms)), -1)::FLOAT AS workspace_connection_latency_95 FROM workspace_agent_stats -- The greater than 0 is to support legacy agents that don't report connection_median_latency_ms. - WHERE workspace_agent_stats.created_at > $1 AND connection_median_latency_ms > 0 + WHERE workspace_agent_stats.created_at > $1 AND connection_median_latency_ms > 0 GROUP BY user_id, agent_id, workspace_id, template_id ), latest_agent_stats AS ( SELECT diff --git a/coderd/database/queries/insights.sql b/coderd/database/queries/insights.sql index 1edb8666e8f1e..dfa236dfbd6d4 100644 --- a/coderd/database/queries/insights.sql +++ b/coderd/database/queries/insights.sql @@ -772,65 +772,10 @@ FROM unique_template_params utp JOIN workspace_build_parameters wbp ON (utp.workspace_build_ids @> ARRAY[wbp.workspace_build_id] AND utp.name = wbp.name) GROUP BY utp.num, utp.template_ids, utp.name, utp.type, utp.display_name, utp.description, utp.options, wbp.value; --- name: GetUserStatusCountsByDay :many -WITH dates AS ( - -- Generate a series of dates between start and end - SELECT - day::date - FROM - generate_series( - date_trunc('day', @start_time::timestamptz), - date_trunc('day', @end_time::timestamptz), - '1 day'::interval - ) AS day -), -all_users AS ( - -- Get all users who have had any status changes in or before the range - SELECT DISTINCT user_id - FROM user_status_changes - WHERE changed_at <= @end_time::timestamptz -), -initial_statuses AS ( - -- Get the status of each user right before each day - SELECT DISTINCT ON (user_id, day) - user_id, - day, - new_status as status - FROM - all_users - CROSS JOIN - dates - LEFT JOIN LATERAL ( - SELECT new_status, changed_at - FROM user_status_changes - WHERE user_status_changes.user_id = all_users.user_id - AND changed_at < day + interval '1 day' - ORDER BY changed_at DESC - LIMIT 1 - ) changes ON true - WHERE changes.new_status IS NOT NULL -), -daily_status AS ( - SELECT - d.day, - i.status, - i.user_id - FROM - dates d - LEFT JOIN - initial_statuses i ON i.day = d.day -) +-- name: GetUserStatusChanges :many SELECT - day, - status, - COUNT(*) AS count -FROM - daily_status -WHERE - status IS NOT NULL -GROUP BY - day, - status -ORDER BY - day ASC, - status ASC; + * +FROM user_status_changes +WHERE changed_at >= @start_time::timestamptz + AND changed_at < @end_time::timestamptz +ORDER BY changed_at; diff --git a/coderd/insights.go b/coderd/insights.go index d5faacee90bd5..b7b2122b4feb7 100644 --- a/coderd/insights.go +++ b/coderd/insights.go @@ -292,6 +292,73 @@ func (api *API) insightsUserLatency(rw http.ResponseWriter, r *http.Request) { httpapi.Write(ctx, rw, http.StatusOK, resp) } +// @Summary Get insights about user status counts over time +// @ID get-insights-about-user-status-counts-over-time +// @Security CoderSessionToken +// @Produce json +// @Tags Insights +// @Param tz_offset query int true "Time-zone offset (e.g. -2)" +// @Success 200 {object} codersdk.GetUserStatusChangesResponse +// @Router /insights/user-status-counts-over-time [get] +func (api *API) insightsUserStatusCountsOverTime(rw http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + p := httpapi.NewQueryParamParser() + vals := r.URL.Query() + tzOffset := p.Int(vals, 0, "tz_offset") + p.ErrorExcessParams(vals) + + if len(p.Errors) > 0 { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Query parameters have invalid values.", + Validations: p.Errors, + }) + return + } + + loc := time.FixedZone("", tzOffset*3600) + // If the time is 14:01 or 14:31, we still want to include all the + // data between 14:00 and 15:00. Our rollups buckets are 30 minutes + // so this works nicely. It works just as well for 23:59 as well. + nextHourInLoc := time.Now().In(loc).Truncate(time.Hour).Add(time.Hour) + // Always return 60 days of data (2 months). + sixtyDaysAgo := nextHourInLoc.In(loc).Truncate(24*time.Hour).AddDate(0, 0, -60) + + rows, err := api.Database.GetUserStatusChanges(ctx, database.GetUserStatusChangesParams{ + StartTime: sixtyDaysAgo, + EndTime: nextHourInLoc, + }) + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error fetching user status counts over time.", + Detail: err.Error(), + }) + return + } + + resp := codersdk.GetUserStatusChangesResponse{ + StatusCounts: make(map[codersdk.UserStatus][]codersdk.UserStatusChangeCount), + } + + slices.SortFunc(rows, func(a, b database.UserStatusChange) int { + return a.ChangedAt.Compare(b.ChangedAt) + }) + + for _, row := range rows { + date := row.ChangedAt.Truncate(24 * time.Hour) + status := codersdk.UserStatus(row.NewStatus) + if _, ok := resp.StatusCounts[status]; !ok { + resp.StatusCounts[status] = make([]codersdk.UserStatusChangeCount, 0) + } + resp.StatusCounts[status] = append(resp.StatusCounts[status], codersdk.UserStatusChangeCount{ + Date: date, + Count: 1, + }) + } + + httpapi.Write(ctx, rw, http.StatusOK, resp) +} + // @Summary Get insights about templates // @ID get-insights-about-templates // @Security CoderSessionToken diff --git a/codersdk/insights.go b/codersdk/insights.go index c9e708de8f34a..b3a3cc728378a 100644 --- a/codersdk/insights.go +++ b/codersdk/insights.go @@ -282,3 +282,34 @@ func (c *Client) TemplateInsights(ctx context.Context, req TemplateInsightsReque var result TemplateInsightsResponse return result, json.NewDecoder(resp.Body).Decode(&result) } + +type GetUserStatusChangesResponse struct { + StatusCounts map[UserStatus][]UserStatusChangeCount `json:"status_counts"` +} + +type UserStatusChangeCount struct { + Date time.Time `json:"date" format:"date-time"` + Count int64 `json:"count" example:"10"` +} + +type GetUserStatusChangesRequest struct { + Offset time.Time `json:"offset" format:"date-time"` +} + +func (c *Client) GetUserStatusChanges(ctx context.Context, req GetUserStatusChangesRequest) (GetUserStatusChangesResponse, error) { + qp := url.Values{} + qp.Add("offset", req.Offset.Format(insightsTimeLayout)) + + reqURL := fmt.Sprintf("/api/v2/insights/user-status-counts-over-time?%s", qp.Encode()) + resp, err := c.Request(ctx, http.MethodGet, reqURL, nil) + if err != nil { + return GetUserStatusChangesResponse{}, xerrors.Errorf("make request: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return GetUserStatusChangesResponse{}, ReadBodyAsError(resp) + } + var result GetUserStatusChangesResponse + return result, json.NewDecoder(resp.Body).Decode(&result) +} diff --git a/docs/reference/api/insights.md b/docs/reference/api/insights.md index e59d74ec6c7f8..f4f00ec24c058 100644 --- a/docs/reference/api/insights.md +++ b/docs/reference/api/insights.md @@ -260,3 +260,53 @@ curl -X GET http://coder-server:8080/api/v2/insights/user-latency?start_time=201 | 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | [codersdk.UserLatencyInsightsResponse](schemas.md#codersdkuserlatencyinsightsresponse) | To perform this operation, you must be authenticated. [Learn more](authentication.md). + +## Get insights about user status counts over time + +### Code samples + +```shell +# Example request using curl +curl -X GET http://coder-server:8080/api/v2/insights/user-status-counts-over-time?tz_offset=0 \ + -H 'Accept: application/json' \ + -H 'Coder-Session-Token: API_KEY' +``` + +`GET /insights/user-status-counts-over-time` + +### Parameters + +| Name | In | Type | Required | Description | +| ----------- | ----- | ------- | -------- | -------------------------- | +| `tz_offset` | query | integer | true | Time-zone offset (e.g. -2) | + +### Example responses + +> 200 Response + +```json +{ + "status_counts": { + "property1": [ + { + "count": 10, + "date": "2019-08-24T14:15:22Z" + } + ], + "property2": [ + { + "count": 10, + "date": "2019-08-24T14:15:22Z" + } + ] + } +} +``` + +### Responses + +| Status | Meaning | Description | Schema | +| ------ | ------------------------------------------------------- | ----------- | ---------------------------------------------------------------------------------------- | +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | [codersdk.GetUserStatusChangesResponse](schemas.md#codersdkgetuserstatuschangesresponse) | + +To perform this operation, you must be authenticated. [Learn more](authentication.md). diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index b6874bc5b1bc9..a735163e2bc57 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -3000,6 +3000,34 @@ Git clone makes use of this by parsing the URL from: 'Username for "https://gith |-------|--------|----------|--------------|-------------| | `key` | string | false | | | +## codersdk.GetUserStatusChangesResponse + +```json +{ + "status_counts": { + "property1": [ + { + "count": 10, + "date": "2019-08-24T14:15:22Z" + } + ], + "property2": [ + { + "count": 10, + "date": "2019-08-24T14:15:22Z" + } + ] + } +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +| ------------------ | ------------------------------------------------------------------------- | -------- | ------------ | ----------- | +| `status_counts` | object | false | | | +| » `[any property]` | array of [codersdk.UserStatusChangeCount](#codersdkuserstatuschangecount) | false | | | + ## codersdk.GetUsersResponse ```json @@ -6724,6 +6752,22 @@ If the schedule is empty, the user will be updated to use the default schedule.| | `dormant` | | `suspended` | +## codersdk.UserStatusChangeCount + +```json +{ + "count": 10, + "date": "2019-08-24T14:15:22Z" +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +| ------- | ------- | -------- | ------------ | ----------- | +| `count` | integer | false | | | +| `date` | string | false | | | + ## codersdk.ValidateUserPasswordRequest ```json diff --git a/site/src/api/api.ts b/site/src/api/api.ts index 6b0e685b177eb..16c0506774295 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -2097,6 +2097,19 @@ class ApiMethods { return response.data; }; + getInsightsUserStatusCountsOverTime = async ( + offset = Math.trunc(new Date().getTimezoneOffset() / 60), + ): Promise => { + const searchParams = new URLSearchParams({ + offset: offset.toString(), + }); + const response = await this.axios.get( + `/api/v2/insights/user-status-counts-over-time?${searchParams}`, + ); + + return response.data; + }; + getHealth = async (force = false) => { const params = new URLSearchParams({ force: force.toString() }); const response = await this.axios.get( diff --git a/site/src/api/queries/insights.ts b/site/src/api/queries/insights.ts index a7044a2f2469f..8f56b5982cd84 100644 --- a/site/src/api/queries/insights.ts +++ b/site/src/api/queries/insights.ts @@ -20,3 +20,10 @@ export const insightsUserActivity = (params: InsightsParams) => { queryFn: () => API.getInsightsUserActivity(params), }; }; + +export const userStatusCountsOverTime = () => { + return { + queryKey: ["userStatusCountsOverTime"], + queryFn: () => API.getInsightsUserStatusCountsOverTime(), + }; +}; diff --git a/site/src/api/queries/users.ts b/site/src/api/queries/users.ts index 77d879abe3258..833d88e6baeef 100644 --- a/site/src/api/queries/users.ts +++ b/site/src/api/queries/users.ts @@ -9,7 +9,6 @@ import type { UpdateUserProfileRequest, User, UsersRequest, - ValidateUserPasswordRequest, } from "api/typesGenerated"; import { type MetadataState, diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 4956de8691ed7..1880265af73b9 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -882,6 +882,16 @@ export interface GenerateAPIKeyResponse { readonly key: string; } +// From codersdk/insights.go +export interface GetUserStatusChangesRequest { + readonly offset: string; +} + +// From codersdk/insights.go +export interface GetUserStatusChangesResponse { + readonly status_counts: Record; +} + // From codersdk/users.go export interface GetUsersResponse { readonly users: readonly User[]; @@ -2690,6 +2700,12 @@ export interface UserRoles { // From codersdk/users.go export type UserStatus = "active" | "dormant" | "suspended"; +// From codersdk/insights.go +export interface UserStatusChangeCount { + readonly date: string; + readonly count: number; +} + export const UserStatuses: UserStatus[] = ["active", "dormant", "suspended"]; // From codersdk/users.go diff --git a/site/src/components/ActiveUserChart/ActiveUserChart.stories.tsx b/site/src/components/ActiveUserChart/ActiveUserChart.stories.tsx index 4f28d7243a0bf..b77886b63fd2a 100644 --- a/site/src/components/ActiveUserChart/ActiveUserChart.stories.tsx +++ b/site/src/components/ActiveUserChart/ActiveUserChart.stories.tsx @@ -5,14 +5,19 @@ const meta: Meta = { title: "components/ActiveUserChart", component: ActiveUserChart, args: { - data: [ - { date: "1/1/2024", amount: 5 }, - { date: "1/2/2024", amount: 6 }, - { date: "1/3/2024", amount: 7 }, - { date: "1/4/2024", amount: 8 }, - { date: "1/5/2024", amount: 9 }, - { date: "1/6/2024", amount: 10 }, - { date: "1/7/2024", amount: 11 }, + series: [ + { + label: "Daily", + data: [ + { date: "1/1/2024", amount: 5 }, + { date: "1/2/2024", amount: 6 }, + { date: "1/3/2024", amount: 7 }, + { date: "1/4/2024", amount: 8 }, + { date: "1/5/2024", amount: 9 }, + { date: "1/6/2024", amount: 10 }, + { date: "1/7/2024", amount: 11 }, + ], + }, ], interval: "day", }, @@ -22,3 +27,51 @@ export default meta; type Story = StoryObj; export const Example: Story = {}; + +export const MultipleSeries: Story = { + args: { + series: [ + { + label: "Active", + data: [ + { date: "1/1/2024", amount: 150 }, + { date: "1/2/2024", amount: 165 }, + { date: "1/3/2024", amount: 180 }, + { date: "1/4/2024", amount: 155 }, + { date: "1/5/2024", amount: 190 }, + { date: "1/6/2024", amount: 200 }, + { date: "1/7/2024", amount: 210 }, + ], + color: "green", + }, + { + label: "Dormant", + data: [ + { date: "1/1/2024", amount: 80 }, + { date: "1/2/2024", amount: 82 }, + { date: "1/3/2024", amount: 85 }, + { date: "1/4/2024", amount: 88 }, + { date: "1/5/2024", amount: 90 }, + { date: "1/6/2024", amount: 92 }, + { date: "1/7/2024", amount: 95 }, + ], + color: "grey", + }, + { + label: "Suspended", + data: [ + { date: "1/1/2024", amount: 20 }, + { date: "1/2/2024", amount: 22 }, + { date: "1/3/2024", amount: 25 }, + { date: "1/4/2024", amount: 23 }, + { date: "1/5/2024", amount: 28 }, + { date: "1/6/2024", amount: 30 }, + { date: "1/7/2024", amount: 32 }, + ], + color: "red", + }, + ], + interval: "day", + userLimit: 100, + }, +}; diff --git a/site/src/components/ActiveUserChart/ActiveUserChart.tsx b/site/src/components/ActiveUserChart/ActiveUserChart.tsx index 41345ea8f03f8..10acb6ec9fc90 100644 --- a/site/src/components/ActiveUserChart/ActiveUserChart.tsx +++ b/site/src/components/ActiveUserChart/ActiveUserChart.tsx @@ -1,5 +1,7 @@ import "chartjs-adapter-date-fns"; import { useTheme } from "@emotion/react"; +import LaunchOutlined from "@mui/icons-material/LaunchOutlined"; +import Button from "@mui/material/Button"; import { CategoryScale, Chart as ChartJS, @@ -14,6 +16,7 @@ import { Tooltip, defaults, } from "chart.js"; +import annotationPlugin from "chartjs-plugin-annotation"; import { HelpTooltip, HelpTooltipContent, @@ -35,32 +38,51 @@ ChartJS.register( Title, Tooltip, Legend, + annotationPlugin, ); -export interface ActiveUserChartProps { +export interface DataSeries { + label?: string; data: readonly { date: string; amount: number }[]; + color?: string; // Optional custom color +} + +export interface ActiveUserChartProps { + series: DataSeries[]; + userLimit?: number; interval: "day" | "week"; } export const ActiveUserChart: FC = ({ - data, + series, + userLimit, interval, }) => { const theme = useTheme(); - const labels = data.map((val) => dayjs(val.date).format("YYYY-MM-DD")); - const chartData = data.map((val) => val.amount); - defaults.font.family = theme.typography.fontFamily as string; defaults.color = theme.palette.text.secondary; const options: ChartOptions<"line"> = { responsive: true, animation: false, + interaction: { + mode: "index", + }, plugins: { - legend: { - display: false, - }, + legend: + series.length > 1 + ? { + display: false, + position: "top" as const, + labels: { + usePointStyle: true, + pointStyle: "line", + }, + } + : { + display: false, + }, tooltip: { displayColors: false, callbacks: { @@ -70,6 +92,24 @@ export const ActiveUserChart: FC = ({ }, }, }, + annotation: { + annotations: [ + { + type: "line", + scaleID: "y", + value: userLimit, + borderColor: "white", + borderWidth: 2, + label: { + content: "Active User limit", + color: theme.palette.primary.contrastText, + display: true, + textStrokeWidth: 2, + textStrokeColor: theme.palette.background.paper, + }, + }, + ], + }, }, scales: { y: { @@ -78,11 +118,12 @@ export const ActiveUserChart: FC = ({ ticks: { precision: 0, }, + stacked: true, }, x: { grid: { color: theme.palette.divider }, ticks: { - stepSize: data.length > 10 ? 2 : undefined, + stepSize: series[0].data.length > 10 ? 2 : undefined, }, type: "time", time: { @@ -97,16 +138,16 @@ export const ActiveUserChart: FC = ({ + dayjs(val.date).format("YYYY-MM-DD"), + ), + datasets: series.map((s) => ({ + label: s.label, + data: s.data.map((val) => val.amount), + pointBackgroundColor: s.color || theme.roles.active.outline, + pointBorderColor: s.color || theme.roles.active.outline, + borderColor: s.color || theme.roles.active.outline, + })), }} options={options} /> @@ -120,11 +161,13 @@ type ActiveUsersTitleProps = { export const ActiveUsersTitle: FC = ({ interval }) => { return (
- {interval === "day" ? "Daily" : "Weekly"} Active Users + {interval === "day" ? "Daily" : "Weekly"} User Activity - How do we calculate active users? + + How do we calculate user activity? + When a connection is initiated to a user's workspace they are considered an active user. e.g. apps, web terminal, SSH. This is for @@ -136,3 +179,39 @@ export const ActiveUsersTitle: FC = ({ interval }) => {
); }; + +export type UserStatusTitleProps = { + interval: "day" | "week"; +}; + +export const UserStatusTitle: FC = ({ interval }) => { + return ( +
+ {interval === "day" ? "Daily" : "Weekly"} User Status + + + + What are user statuses? + + + Active users count towards your license consumption. Dormant or + suspended users do not. Any user who has logged into the coder + platform within the last 90 days is considered active. + + + + + +
+ ); +}; diff --git a/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx b/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx index 2b094cbf89b26..3de614a42ac39 100644 --- a/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx +++ b/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx @@ -1,6 +1,6 @@ import { deploymentDAUs } from "api/queries/deployment"; -import { entitlements } from "api/queries/entitlements"; import { availableExperiments, experiments } from "api/queries/experiments"; +import { userStatusCountsOverTime } from "api/queries/insights"; import { useEmbeddedMetadata } from "hooks/useEmbeddedMetadata"; import { useDeploymentSettings } from "modules/management/DeploymentSettingsProvider"; import type { FC } from "react"; @@ -15,9 +15,8 @@ const GeneralSettingsPage: FC = () => { const safeExperimentsQuery = useQuery(availableExperiments()); const { metadata } = useEmbeddedMetadata(); - const entitlementsQuery = useQuery(entitlements(metadata.entitlements)); const enabledExperimentsQuery = useQuery(experiments(metadata.experiments)); - + const userStatusCountsOverTimeQuery = useQuery(userStatusCountsOverTime()); const safeExperiments = safeExperimentsQuery.data?.safe ?? []; const invalidExperiments = enabledExperimentsQuery.data?.filter((exp) => { @@ -33,9 +32,9 @@ const GeneralSettingsPage: FC = () => { deploymentOptions={deploymentConfig.options} deploymentDAUs={deploymentDAUsQuery.data} deploymentDAUsError={deploymentDAUsQuery.error} - entitlements={entitlementsQuery.data} invalidExperiments={invalidExperiments} safeExperiments={safeExperiments} + userStatusCountsOverTime={userStatusCountsOverTimeQuery.data} /> ); diff --git a/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx b/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx index 05ed426d5dcc9..78291ee03b4d8 100644 --- a/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx +++ b/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx @@ -1,9 +1,5 @@ import type { Meta, StoryObj } from "@storybook/react"; -import { - MockDeploymentDAUResponse, - MockEntitlementsWithUserLimit, - mockApiError, -} from "testHelpers/entities"; +import { MockDeploymentDAUResponse, mockApiError } from "testHelpers/entities"; import { GeneralSettingsPageView } from "./GeneralSettingsPageView"; const meta: Meta = { @@ -42,7 +38,100 @@ const meta: Meta = { deploymentDAUs: MockDeploymentDAUResponse, invalidExperiments: [], safeExperiments: [], - entitlements: undefined, + userStatusCountsOverTime: { + status_counts: { + active: [ + { + date: "1/1/2024", + count: 1, + }, + { + date: "1/2/2024", + count: 8, + }, + { + date: "1/3/2024", + count: 8, + }, + { + date: "1/4/2024", + count: 6, + }, + { + date: "1/5/2024", + count: 6, + }, + { + date: "1/6/2024", + count: 6, + }, + { + date: "1/7/2024", + count: 6, + }, + ], + dormant: [ + { + date: "1/1/2024", + count: 0, + }, + { + date: "1/2/2024", + count: 3, + }, + { + date: "1/3/2024", + count: 3, + }, + { + date: "1/4/2024", + count: 3, + }, + { + date: "1/5/2024", + count: 3, + }, + { + date: "1/6/2024", + count: 3, + }, + { + date: "1/7/2024", + count: 3, + }, + ], + suspended: [ + { + date: "1/1/2024", + count: 0, + }, + { + date: "1/2/2024", + count: 0, + }, + { + date: "1/3/2024", + count: 0, + }, + { + date: "1/4/2024", + count: 2, + }, + { + date: "1/5/2024", + count: 2, + }, + { + date: "1/6/2024", + count: 2, + }, + { + date: "1/7/2024", + count: 2, + }, + ], + }, + }, }, }; @@ -138,73 +227,26 @@ export const invalidExperimentsEnabled: Story = { }, }; -export const WithLicenseUtilization: Story = { - args: { - entitlements: { - ...MockEntitlementsWithUserLimit, - features: { - ...MockEntitlementsWithUserLimit.features, - user_limit: { - ...MockEntitlementsWithUserLimit.features.user_limit, - enabled: true, - actual: 75, - limit: 100, - entitlement: "entitled", - }, - }, - }, - }, +export const UnlicensedInstallation: Story = { + args: {}, }; -export const HighLicenseUtilization: Story = { - args: { - entitlements: { - ...MockEntitlementsWithUserLimit, - features: { - ...MockEntitlementsWithUserLimit.features, - user_limit: { - ...MockEntitlementsWithUserLimit.features.user_limit, - enabled: true, - actual: 95, - limit: 100, - entitlement: "entitled", - }, - }, - }, - }, +export const LicensedWithNoUserLimit: Story = { + args: {}, }; -export const ExceedsLicenseUtilization: Story = { +export const LicensedWithPlentyOfSpareLicenses: Story = { args: { - entitlements: { - ...MockEntitlementsWithUserLimit, - features: { - ...MockEntitlementsWithUserLimit.features, - user_limit: { - ...MockEntitlementsWithUserLimit.features.user_limit, - enabled: true, - actual: 100, - limit: 95, - entitlement: "entitled", - }, - }, - }, + activeUserLimit: 100, }, }; -export const NoLicenseLimit: Story = { + +export const TotalUsersExceedsLicenseButNotActiveUsers: Story = { args: { - entitlements: { - ...MockEntitlementsWithUserLimit, - features: { - ...MockEntitlementsWithUserLimit.features, - user_limit: { - ...MockEntitlementsWithUserLimit.features.user_limit, - enabled: false, - actual: 0, - limit: 0, - entitlement: "entitled", - }, - }, - }, + activeUserLimit: 8, }, }; + +export const ManyUsers: Story = { + args: {}, +}; diff --git a/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPageView.tsx b/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPageView.tsx index df5550d70e965..bf663fecaa945 100644 --- a/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPageView.tsx +++ b/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPageView.tsx @@ -1,14 +1,15 @@ import AlertTitle from "@mui/material/AlertTitle"; -import LinearProgress from "@mui/material/LinearProgress"; import type { DAUsResponse, - Entitlements, Experiments, + GetUserStatusChangesResponse, SerpentOption, } from "api/typesGenerated"; import { ActiveUserChart, ActiveUsersTitle, + type DataSeries, + UserStatusTitle, } from "components/ActiveUserChart/ActiveUserChart"; import { ErrorAlert } from "components/Alert/ErrorAlert"; import { SettingsHeader } from "components/SettingsHeader/SettingsHeader"; @@ -24,7 +25,8 @@ export type GeneralSettingsPageViewProps = { deploymentOptions: SerpentOption[]; deploymentDAUs?: DAUsResponse; deploymentDAUsError: unknown; - entitlements: Entitlements | undefined; + userStatusCountsOverTime?: GetUserStatusChangesResponse; + activeUserLimit?: number; readonly invalidExperiments: Experiments | string[]; readonly safeExperiments: Experiments | string[]; }; @@ -33,16 +35,29 @@ export const GeneralSettingsPageView: FC = ({ deploymentOptions, deploymentDAUs, deploymentDAUsError, - entitlements, + userStatusCountsOverTime, + activeUserLimit, safeExperiments, invalidExperiments, }) => { - const licenseUtilizationPercentage = - entitlements?.features?.user_limit?.actual && - entitlements?.features?.user_limit?.limit - ? entitlements.features.user_limit.actual / - entitlements.features.user_limit.limit - : undefined; + const colors: Record = { + active: "green", + dormant: "grey", + deleted: "red", + }; + let series: DataSeries[] = []; + if (userStatusCountsOverTime?.status_counts) { + series = Object.entries(userStatusCountsOverTime.status_counts).map( + ([status, counts]) => ({ + label: status, + data: counts.map((count) => ({ + date: count.date.toString(), + amount: count.count, + })), + color: colors[status], + }), + ); + } return ( <> = ({ {Boolean(deploymentDAUsError) && ( )} - {deploymentDAUs && ( -
- }> - - -
+ {series.length && ( + }> + + )} - {licenseUtilizationPercentage && ( - - }> + - - {Math.round(licenseUtilizationPercentage * 100)}% used ( - {entitlements!.features.user_limit.actual}/ - {entitlements!.features.user_limit.limit} users) - )} {invalidExperiments.length > 0 && ( diff --git a/site/src/pages/TemplatePage/TemplateInsightsPage/TemplateInsightsPage.tsx b/site/src/pages/TemplatePage/TemplateInsightsPage/TemplateInsightsPage.tsx index 097b8fce513e7..2b873c325e274 100644 --- a/site/src/pages/TemplatePage/TemplateInsightsPage/TemplateInsightsPage.tsx +++ b/site/src/pages/TemplatePage/TemplateInsightsPage/TemplateInsightsPage.tsx @@ -258,10 +258,14 @@ const ActiveUsersPanel: FC = ({ {data && data.length > 0 && ( ({ - amount: d.active_users, - date: d.start_time, - }))} + series={[ + { + data: data.map((d) => ({ + amount: d.active_users, + date: d.start_time, + })), + }, + ]} /> )} From 0b3e0e68253f680665eeaf412616c63fae43f194 Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Mon, 23 Dec 2024 09:57:56 +0000 Subject: [PATCH 08/32] rename migration --- ...tatus_changes.down.sql => 000280_user_status_changes.down.sql} | 0 ...er_status_changes.up.sql => 000280_user_status_changes.up.sql} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename coderd/database/migrations/{000279_user_status_changes.down.sql => 000280_user_status_changes.down.sql} (100%) rename coderd/database/migrations/{000279_user_status_changes.up.sql => 000280_user_status_changes.up.sql} (100%) diff --git a/coderd/database/migrations/000279_user_status_changes.down.sql b/coderd/database/migrations/000280_user_status_changes.down.sql similarity index 100% rename from coderd/database/migrations/000279_user_status_changes.down.sql rename to coderd/database/migrations/000280_user_status_changes.down.sql diff --git a/coderd/database/migrations/000279_user_status_changes.up.sql b/coderd/database/migrations/000280_user_status_changes.up.sql similarity index 100% rename from coderd/database/migrations/000279_user_status_changes.up.sql rename to coderd/database/migrations/000280_user_status_changes.up.sql From 6462cc26380b1eaddebac202e29397e90077e745 Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Tue, 24 Dec 2024 08:49:05 +0000 Subject: [PATCH 09/32] move aggregation logic for GetUserStatusChanges into the SQL --- coderd/database/dbauthz/dbauthz.go | 2 +- coderd/database/dbmem/dbmem.go | 21 ++++++- coderd/database/dbmetrics/querymetrics.go | 2 +- coderd/database/dbmock/dbmock.go | 4 +- coderd/database/querier.go | 2 +- coderd/database/queries.sql.go | 74 ++++++++++++++++------- coderd/database/queries/insights.sql | 37 ++++++++++-- coderd/insights.go | 9 +-- 8 files changed, 108 insertions(+), 43 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index f0201459c27c8..7dff12529fe8a 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -2421,7 +2421,7 @@ func (q *querier) GetUserNotificationPreferences(ctx context.Context, userID uui return q.db.GetUserNotificationPreferences(ctx, userID) } -func (q *querier) GetUserStatusChanges(ctx context.Context, arg database.GetUserStatusChangesParams) ([]database.UserStatusChange, error) { +func (q *querier) GetUserStatusChanges(ctx context.Context, arg database.GetUserStatusChangesParams) ([]database.GetUserStatusChangesRow, error) { if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceUser); err != nil { return nil, err } diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index d236ec69a2e32..58a29158ea392 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -5671,7 +5671,7 @@ func (q *FakeQuerier) GetUserNotificationPreferences(_ context.Context, userID u return out, nil } -func (q *FakeQuerier) GetUserStatusChanges(_ context.Context, arg database.GetUserStatusChangesParams) ([]database.UserStatusChange, error) { +func (q *FakeQuerier) GetUserStatusChanges(_ context.Context, arg database.GetUserStatusChangesParams) ([]database.GetUserStatusChangesRow, error) { q.mutex.RLock() defer q.mutex.RUnlock() @@ -5680,12 +5680,27 @@ func (q *FakeQuerier) GetUserStatusChanges(_ context.Context, arg database.GetUs return nil, err } - result := make([]database.UserStatusChange, 0) + result := make([]database.GetUserStatusChangesRow, 0) for _, change := range q.userStatusChanges { if change.ChangedAt.Before(arg.StartTime) || change.ChangedAt.After(arg.EndTime) { continue } - result = append(result, change) + if !slices.ContainsFunc(result, func(r database.GetUserStatusChangesRow) bool { + return r.ChangedAt.Equal(change.ChangedAt) && r.NewStatus == change.NewStatus + }) { + result = append(result, database.GetUserStatusChangesRow{ + NewStatus: change.NewStatus, + ChangedAt: change.ChangedAt, + Count: 1, + }) + } else { + for i, r := range result { + if r.ChangedAt.Equal(change.ChangedAt) && r.NewStatus == change.NewStatus { + result[i].Count++ + break + } + } + } } return result, nil diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index 7c001489e3a7e..f3866cbd6a482 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -1344,7 +1344,7 @@ func (m queryMetricsStore) GetUserNotificationPreferences(ctx context.Context, u return r0, r1 } -func (m queryMetricsStore) GetUserStatusChanges(ctx context.Context, arg database.GetUserStatusChangesParams) ([]database.UserStatusChange, error) { +func (m queryMetricsStore) GetUserStatusChanges(ctx context.Context, arg database.GetUserStatusChangesParams) ([]database.GetUserStatusChangesRow, error) { start := time.Now() r0, r1 := m.s.GetUserStatusChanges(ctx, arg) m.queryLatencies.WithLabelValues("GetUserStatusChanges").Observe(time.Since(start).Seconds()) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index 6391f4a2277a1..6abc14205a22f 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -2826,10 +2826,10 @@ func (mr *MockStoreMockRecorder) GetUserNotificationPreferences(arg0, arg1 any) } // GetUserStatusChanges mocks base method. -func (m *MockStore) GetUserStatusChanges(arg0 context.Context, arg1 database.GetUserStatusChangesParams) ([]database.UserStatusChange, error) { +func (m *MockStore) GetUserStatusChanges(arg0 context.Context, arg1 database.GetUserStatusChangesParams) ([]database.GetUserStatusChangesRow, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetUserStatusChanges", arg0, arg1) - ret0, _ := ret[0].([]database.UserStatusChange) + ret0, _ := ret[0].([]database.GetUserStatusChangesRow) ret1, _ := ret[1].(error) return ret0, ret1 } diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 332a2c2c535bb..0e3459882c73b 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -289,7 +289,7 @@ type sqlcQuerier interface { GetUserLinkByUserIDLoginType(ctx context.Context, arg GetUserLinkByUserIDLoginTypeParams) (UserLink, error) GetUserLinksByUserID(ctx context.Context, userID uuid.UUID) ([]UserLink, error) GetUserNotificationPreferences(ctx context.Context, userID uuid.UUID) ([]NotificationPreference, error) - GetUserStatusChanges(ctx context.Context, arg GetUserStatusChangesParams) ([]UserStatusChange, error) + GetUserStatusChanges(ctx context.Context, arg GetUserStatusChangesParams) ([]GetUserStatusChangesRow, error) GetUserWorkspaceBuildParameters(ctx context.Context, arg GetUserWorkspaceBuildParametersParams) ([]GetUserWorkspaceBuildParametersRow, error) // This will never return deleted users. GetUsers(ctx context.Context, arg GetUsersParams) ([]GetUsersRow, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 85559b07eebd1..9cf778c1b022c 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3094,13 +3094,40 @@ func (q *sqlQuerier) GetUserLatencyInsights(ctx context.Context, arg GetUserLate return items, nil } -const GetUserStatusChanges = `-- name: GetUserStatusChanges :many +const getUserStatusChanges = `-- name: GetUserStatusChanges :many +WITH last_status_per_day AS ( + -- First get the last status change for each user for each day + SELECT DISTINCT ON (date_trunc('day', changed_at), user_id) + date_trunc('day', changed_at)::timestamptz AS date, + new_status, + user_id + FROM user_status_changes + WHERE changed_at >= $1::timestamptz + AND changed_at < $2::timestamptz + ORDER BY + date_trunc('day', changed_at), + user_id, + changed_at DESC -- This ensures we get the last status for each day +), +daily_counts AS ( + -- Then count unique users per status per day + SELECT + date, + new_status, + COUNT(*) AS count + FROM last_status_per_day + GROUP BY + date, + new_status +) SELECT - id, user_id, new_status, changed_at -FROM user_status_changes -WHERE changed_at >= $1::timestamptz - AND changed_at < $2::timestamptz -ORDER BY changed_at + date::timestamptz AS changed_at, + new_status, + count::bigint +FROM daily_counts +ORDER BY + new_status ASC, + date ASC ` type GetUserStatusChangesParams struct { @@ -3108,21 +3135,22 @@ type GetUserStatusChangesParams struct { EndTime time.Time `db:"end_time" json:"end_time"` } -func (q *sqlQuerier) GetUserStatusChanges(ctx context.Context, arg GetUserStatusChangesParams) ([]UserStatusChange, error) { - rows, err := q.db.QueryContext(ctx, GetUserStatusChanges, arg.StartTime, arg.EndTime) +type GetUserStatusChangesRow struct { + ChangedAt time.Time `db:"changed_at" json:"changed_at"` + NewStatus UserStatus `db:"new_status" json:"new_status"` + Count int64 `db:"count" json:"count"` +} + +func (q *sqlQuerier) GetUserStatusChanges(ctx context.Context, arg GetUserStatusChangesParams) ([]GetUserStatusChangesRow, error) { + rows, err := q.db.QueryContext(ctx, getUserStatusChanges, arg.StartTime, arg.EndTime) if err != nil { return nil, err } defer rows.Close() - var items []UserStatusChange + var items []GetUserStatusChangesRow for rows.Next() { - var i UserStatusChange - if err := rows.Scan( - &i.ID, - &i.UserID, - &i.NewStatus, - &i.ChangedAt, - ); err != nil { + var i GetUserStatusChangesRow + if err := rows.Scan(&i.ChangedAt, &i.NewStatus, &i.Count); err != nil { return nil, err } items = append(items, i) @@ -3432,7 +3460,7 @@ func (q *sqlQuerier) GetJFrogXrayScanByWorkspaceAndAgentID(ctx context.Context, } const upsertJFrogXrayScanByWorkspaceAndAgentID = `-- name: UpsertJFrogXrayScanByWorkspaceAndAgentID :exec -INSERT INTO +INSERT INTO jfrog_xray_scans ( agent_id, workspace_id, @@ -3441,7 +3469,7 @@ INSERT INTO medium, results_url ) -VALUES +VALUES ($1, $2, $3, $4, $5, $6) ON CONFLICT (agent_id, workspace_id) DO UPDATE SET critical = $3, high = $4, medium = $5, results_url = $6 @@ -6316,7 +6344,7 @@ FROM provisioner_keys WHERE organization_id = $1 -AND +AND lower(name) = lower($2) ` @@ -6432,10 +6460,10 @@ WHERE AND -- exclude reserved built-in key id != '00000000-0000-0000-0000-000000000001'::uuid -AND +AND -- exclude reserved user-auth key id != '00000000-0000-0000-0000-000000000002'::uuid -AND +AND -- exclude reserved psk key id != '00000000-0000-0000-0000-000000000003'::uuid ` @@ -8121,7 +8149,7 @@ func (q *sqlQuerier) GetTailnetTunnelPeerIDs(ctx context.Context, srcID uuid.UUI } const updateTailnetPeerStatusByCoordinator = `-- name: UpdateTailnetPeerStatusByCoordinator :exec -UPDATE +UPDATE tailnet_peers SET status = $2 @@ -12734,7 +12762,7 @@ WITH agent_stats AS ( coalesce((PERCENTILE_CONT(0.95) WITHIN GROUP (ORDER BY connection_median_latency_ms)), -1)::FLOAT AS workspace_connection_latency_95 FROM workspace_agent_stats -- The greater than 0 is to support legacy agents that don't report connection_median_latency_ms. - WHERE workspace_agent_stats.created_at > $1 AND connection_median_latency_ms > 0 + WHERE workspace_agent_stats.created_at > $1 AND connection_median_latency_ms > 0 GROUP BY user_id, agent_id, workspace_id, template_id ), latest_agent_stats AS ( SELECT diff --git a/coderd/database/queries/insights.sql b/coderd/database/queries/insights.sql index dfa236dfbd6d4..513ff7eeec1ec 100644 --- a/coderd/database/queries/insights.sql +++ b/coderd/database/queries/insights.sql @@ -773,9 +773,36 @@ JOIN workspace_build_parameters wbp ON (utp.workspace_build_ids @> ARRAY[wbp.wor GROUP BY utp.num, utp.template_ids, utp.name, utp.type, utp.display_name, utp.description, utp.options, wbp.value; -- name: GetUserStatusChanges :many +WITH last_status_per_day AS ( + -- First get the last status change for each user for each day + SELECT DISTINCT ON (date_trunc('day', changed_at), user_id) + date_trunc('day', changed_at)::timestamptz AS date, + new_status, + user_id + FROM user_status_changes + WHERE changed_at >= @start_time::timestamptz + AND changed_at < @end_time::timestamptz + ORDER BY + date_trunc('day', changed_at), + user_id, + changed_at DESC -- This ensures we get the last status for each day +), +daily_counts AS ( + -- Then count unique users per status per day + SELECT + date, + new_status, + COUNT(*) AS count + FROM last_status_per_day + GROUP BY + date, + new_status +) SELECT - * -FROM user_status_changes -WHERE changed_at >= @start_time::timestamptz - AND changed_at < @end_time::timestamptz -ORDER BY changed_at; + date::timestamptz AS changed_at, + new_status, + count::bigint +FROM daily_counts +ORDER BY + new_status ASC, + date ASC; diff --git a/coderd/insights.go b/coderd/insights.go index b7b2122b4feb7..dfa914425f965 100644 --- a/coderd/insights.go +++ b/coderd/insights.go @@ -340,19 +340,14 @@ func (api *API) insightsUserStatusCountsOverTime(rw http.ResponseWriter, r *http StatusCounts: make(map[codersdk.UserStatus][]codersdk.UserStatusChangeCount), } - slices.SortFunc(rows, func(a, b database.UserStatusChange) int { - return a.ChangedAt.Compare(b.ChangedAt) - }) - for _, row := range rows { - date := row.ChangedAt.Truncate(24 * time.Hour) status := codersdk.UserStatus(row.NewStatus) if _, ok := resp.StatusCounts[status]; !ok { resp.StatusCounts[status] = make([]codersdk.UserStatusChangeCount, 0) } resp.StatusCounts[status] = append(resp.StatusCounts[status], codersdk.UserStatusChangeCount{ - Date: date, - Count: 1, + Date: row.ChangedAt, + Count: row.Count, }) } From ccd0cdfc9ceb73c7a6da4f33f0bc8c50eecef2f4 Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Tue, 24 Dec 2024 10:20:09 +0000 Subject: [PATCH 10/32] use window functions for efficiency --- coderd/database/querier_test.go | 57 ++++++++++++----------- coderd/database/queries.sql.go | 68 ++++++++++++++++++++-------- coderd/database/queries/insights.sql | 68 ++++++++++++++++++++-------- 3 files changed, 127 insertions(+), 66 deletions(-) diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index c724789df58ac..1c5d4778f6dcb 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -2258,6 +2258,10 @@ func TestGroupRemovalTrigger(t *testing.T) { func TestGetUserStatusChanges(t *testing.T) { t.Parallel() + now := dbtime.Now() + createdAt := now.Add(-5 * 24 * time.Hour) // 5 days ago + firstTransitionTime := createdAt.Add(2 * 24 * time.Hour) // 3 days ago + secondTransitionTime := firstTransitionTime.Add(2 * 24 * time.Hour) // 1 days ago t.Run("No Users", func(t *testing.T) { t.Parallel() db, _ := dbtestutil.NewDB(t) @@ -2307,8 +2311,6 @@ func TestGetUserStatusChanges(t *testing.T) { ctx := testutil.Context(t, testutil.WaitShort) // Create a user that's been in the specified status for the past 30 days - now := dbtime.Now() - createdAt := now.Add(-29 * 24 * time.Hour) dbgen.User(t, db, database.User{ Status: tc.status, CreatedAt: createdAt, @@ -2324,8 +2326,10 @@ func TestGetUserStatusChanges(t *testing.T) { require.NotEmpty(t, userStatusChanges, "should return results") // We should have an entry for each status change - require.Len(t, userStatusChanges, 1, "should have 1 status change") - require.Equal(t, userStatusChanges[0].NewStatus, tc.status, "should have the correct status") + require.Len(t, userStatusChanges, 5, "should have 1 user * 5 days = 5 rows") + for _, row := range userStatusChanges { + require.Equal(t, row.NewStatus, tc.status, "should have the correct status") + } }) } }) @@ -2393,8 +2397,6 @@ func TestGetUserStatusChanges(t *testing.T) { ctx := testutil.Context(t, testutil.WaitShort) // Create a user that starts with initial status - now := dbtime.Now() - createdAt := now.Add(-5 * 24 * time.Hour) // 5 days ago user := dbgen.User(t, db, database.User{ Status: tc.initialStatus, CreatedAt: createdAt, @@ -2402,11 +2404,10 @@ func TestGetUserStatusChanges(t *testing.T) { }) // After 2 days, change status to target status - statusChangeTime := createdAt.Add(2 * 24 * time.Hour) user, err := db.UpdateUserStatus(ctx, database.UpdateUserStatusParams{ ID: user.ID, Status: tc.targetStatus, - UpdatedAt: statusChangeTime, + UpdatedAt: firstTransitionTime, }) require.NoError(t, err) @@ -2418,10 +2419,15 @@ func TestGetUserStatusChanges(t *testing.T) { require.NoError(t, err) require.NotEmpty(t, userStatusChanges, "should return results") - // We should have an entry for each status change, including the initial status - require.Len(t, userStatusChanges, 2, "should have 2 status changes") - require.Equal(t, userStatusChanges[0].NewStatus, tc.initialStatus, "should have the initial status") - require.Equal(t, userStatusChanges[1].NewStatus, tc.targetStatus, "should have the target status") + // We should have an entry for each status (active, dormant, suspended) for each day + require.Len(t, userStatusChanges, 5, "should have 1 user * 5 days = 5 rows") + for _, row := range userStatusChanges { + if row.ChangedAt.Before(firstTransitionTime) { + require.Equal(t, row.NewStatus, tc.initialStatus, "should have the initial status") + } else { + require.Equal(t, row.NewStatus, tc.targetStatus, "should have the target status") + } + } }) } }) @@ -2606,20 +2612,18 @@ func TestGetUserStatusChanges(t *testing.T) { }) // First transition at 2 days - user1TransitionTime := createdAt.Add(2 * 24 * time.Hour) user1, err := db.UpdateUserStatus(ctx, database.UpdateUserStatusParams{ ID: user1.ID, Status: tc.user1Transition.to, - UpdatedAt: user1TransitionTime, + UpdatedAt: firstTransitionTime, }) require.NoError(t, err) // Second transition at 4 days - user2TransitionTime := createdAt.Add(4 * 24 * time.Hour) user2, err = db.UpdateUserStatus(ctx, database.UpdateUserStatusParams{ ID: user2.ID, Status: tc.user2Transition.to, - UpdatedAt: user2TransitionTime, + UpdatedAt: secondTransitionTime, }) require.NoError(t, err) @@ -2631,16 +2635,17 @@ func TestGetUserStatusChanges(t *testing.T) { require.NoError(t, err) require.NotEmpty(t, userStatusChanges) - // We should have an entry with the correct status changes for each user, including the initial status - require.Len(t, userStatusChanges, 4, "should have 4 status changes") - require.Equal(t, userStatusChanges[0].UserID, user1.ID, "should have the first user") - require.Equal(t, userStatusChanges[0].NewStatus, tc.user1Transition.from, "should have the first user's initial status") - require.Equal(t, userStatusChanges[1].UserID, user1.ID, "should have the first user") - require.Equal(t, userStatusChanges[1].NewStatus, tc.user1Transition.to, "should have the first user's target status") - require.Equal(t, userStatusChanges[2].UserID, user2.ID, "should have the second user") - require.Equal(t, userStatusChanges[2].NewStatus, tc.user2Transition.from, "should have the second user's initial status") - require.Equal(t, userStatusChanges[3].UserID, user2.ID, "should have the second user") - require.Equal(t, userStatusChanges[3].NewStatus, tc.user2Transition.to, "should have the second user's target status") + // Expected counts before, between and after the transitions should match: + for _, row := range userStatusChanges { + switch { + case row.ChangedAt.Before(firstTransitionTime): + require.Equal(t, row.Count, tc.expectedCounts["initial"][row.NewStatus], "should have the correct count before the first transition") + case row.ChangedAt.Before(secondTransitionTime): + require.Equal(t, row.Count, tc.expectedCounts["between"][row.NewStatus], "should have the correct count between the transitions") + case row.ChangedAt.Before(now): + require.Equal(t, row.Count, tc.expectedCounts["final"][row.NewStatus], "should have the correct count after the second transition") + } + } }) } }) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 9cf778c1b022c..2d4d3882b3ac8 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3095,36 +3095,64 @@ func (q *sqlQuerier) GetUserLatencyInsights(ctx context.Context, arg GetUserLate } const getUserStatusChanges = `-- name: GetUserStatusChanges :many -WITH last_status_per_day AS ( - -- First get the last status change for each user for each day - SELECT DISTINCT ON (date_trunc('day', changed_at), user_id) - date_trunc('day', changed_at)::timestamptz AS date, +WITH dates AS ( + SELECT generate_series( + date_trunc('day', $1::timestamptz), + date_trunc('day', $2::timestamptz), + '1 day'::interval + )::timestamptz AS date +), +latest_status_before_range AS ( + -- Get the last status change for each user before our date range + SELECT DISTINCT ON (user_id) + user_id, new_status, - user_id + changed_at FROM user_status_changes - WHERE changed_at >= $1::timestamptz - AND changed_at < $2::timestamptz - ORDER BY - date_trunc('day', changed_at), + WHERE changed_at < (SELECT MIN(date) FROM dates) + ORDER BY user_id, changed_at DESC +), +all_status_changes AS ( + -- Combine status changes before and during our range + SELECT + user_id, + new_status, + changed_at + FROM latest_status_before_range + + UNION ALL + + SELECT user_id, - changed_at DESC -- This ensures we get the last status for each day + new_status, + changed_at + FROM user_status_changes + WHERE changed_at < $2::timestamptz ), daily_counts AS ( - -- Then count unique users per status per day SELECT - date, - new_status, - COUNT(*) AS count - FROM last_status_per_day - GROUP BY - date, - new_status + d.date, + asc1.new_status, + -- For each date and status, count users whose most recent status change + -- (up to that date) matches this status + COUNT(*) FILTER ( + WHERE asc1.changed_at = ( + SELECT MAX(changed_at) + FROM all_status_changes asc2 + WHERE asc2.user_id = asc1.user_id + AND asc2.changed_at <= d.date + ) + )::bigint AS count + FROM dates d + CROSS JOIN all_status_changes asc1 + GROUP BY d.date, asc1.new_status ) SELECT - date::timestamptz AS changed_at, + date AS changed_at, new_status, - count::bigint + count FROM daily_counts +WHERE count > 0 ORDER BY new_status ASC, date ASC diff --git a/coderd/database/queries/insights.sql b/coderd/database/queries/insights.sql index 513ff7eeec1ec..25f46617efeba 100644 --- a/coderd/database/queries/insights.sql +++ b/coderd/database/queries/insights.sql @@ -773,36 +773,64 @@ JOIN workspace_build_parameters wbp ON (utp.workspace_build_ids @> ARRAY[wbp.wor GROUP BY utp.num, utp.template_ids, utp.name, utp.type, utp.display_name, utp.description, utp.options, wbp.value; -- name: GetUserStatusChanges :many -WITH last_status_per_day AS ( - -- First get the last status change for each user for each day - SELECT DISTINCT ON (date_trunc('day', changed_at), user_id) - date_trunc('day', changed_at)::timestamptz AS date, +WITH dates AS ( + SELECT generate_series( + date_trunc('day', @start_time::timestamptz), + date_trunc('day', @end_time::timestamptz), + '1 day'::interval + )::timestamptz AS date +), +latest_status_before_range AS ( + -- Get the last status change for each user before our date range + SELECT DISTINCT ON (user_id) + user_id, new_status, - user_id + changed_at FROM user_status_changes - WHERE changed_at >= @start_time::timestamptz - AND changed_at < @end_time::timestamptz - ORDER BY - date_trunc('day', changed_at), + WHERE changed_at < (SELECT MIN(date) FROM dates) + ORDER BY user_id, changed_at DESC +), +all_status_changes AS ( + -- Combine status changes before and during our range + SELECT + user_id, + new_status, + changed_at + FROM latest_status_before_range + + UNION ALL + + SELECT user_id, - changed_at DESC -- This ensures we get the last status for each day + new_status, + changed_at + FROM user_status_changes + WHERE changed_at < @end_time::timestamptz ), daily_counts AS ( - -- Then count unique users per status per day SELECT - date, - new_status, - COUNT(*) AS count - FROM last_status_per_day - GROUP BY - date, - new_status + d.date, + asc1.new_status, + -- For each date and status, count users whose most recent status change + -- (up to that date) matches this status + COUNT(*) FILTER ( + WHERE asc1.changed_at = ( + SELECT MAX(changed_at) + FROM all_status_changes asc2 + WHERE asc2.user_id = asc1.user_id + AND asc2.changed_at <= d.date + ) + )::bigint AS count + FROM dates d + CROSS JOIN all_status_changes asc1 + GROUP BY d.date, asc1.new_status ) SELECT - date::timestamptz AS changed_at, + date AS changed_at, new_status, - count::bigint + count FROM daily_counts +WHERE count > 0 ORDER BY new_status ASC, date ASC; From 12a274fd364442cec84ab6ffa55e7b905ac29b43 Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Tue, 24 Dec 2024 10:27:40 +0000 Subject: [PATCH 11/32] ensure we use the same time zone as the start_time param --- coderd/database/queries/insights.sql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/coderd/database/queries/insights.sql b/coderd/database/queries/insights.sql index 25f46617efeba..18ea61de32773 100644 --- a/coderd/database/queries/insights.sql +++ b/coderd/database/queries/insights.sql @@ -774,11 +774,11 @@ GROUP BY utp.num, utp.template_ids, utp.name, utp.type, utp.display_name, utp.de -- name: GetUserStatusChanges :many WITH dates AS ( - SELECT generate_series( + SELECT (generate_series( date_trunc('day', @start_time::timestamptz), date_trunc('day', @end_time::timestamptz), '1 day'::interval - )::timestamptz AS date + ) AT TIME ZONE (extract(timezone FROM @start_time::timestamptz)::text || ' minutes'))::timestamptz AS date ), latest_status_before_range AS ( -- Get the last status change for each user before our date range From fcfd76e9a4f3e26c838deac89cc046cbc90e2b19 Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Tue, 24 Dec 2024 10:37:38 +0000 Subject: [PATCH 12/32] ensure we use the same time zone as the start_time param --- coderd/database/queries.sql.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 2d4d3882b3ac8..d8074c57e9a17 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3096,11 +3096,11 @@ func (q *sqlQuerier) GetUserLatencyInsights(ctx context.Context, arg GetUserLate const getUserStatusChanges = `-- name: GetUserStatusChanges :many WITH dates AS ( - SELECT generate_series( + SELECT (generate_series( date_trunc('day', $1::timestamptz), date_trunc('day', $2::timestamptz), '1 day'::interval - )::timestamptz AS date + ) AT TIME ZONE (extract(timezone FROM $1::timestamptz)::text || ' minutes'))::timestamptz AS date ), latest_status_before_range AS ( -- Get the last status change for each user before our date range From 7c0cade5e785dc0a554a9a53c78b4b3d436f6f43 Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Tue, 24 Dec 2024 14:29:32 +0000 Subject: [PATCH 13/32] make gen --- site/src/api/typesGenerated.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 1880265af73b9..6faaf18dadadc 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -884,12 +884,12 @@ export interface GenerateAPIKeyResponse { // From codersdk/insights.go export interface GetUserStatusChangesRequest { - readonly offset: string; + readonly offset: string; } // From codersdk/insights.go export interface GetUserStatusChangesResponse { - readonly status_counts: Record; + readonly status_counts: Record; } // From codersdk/users.go @@ -2702,8 +2702,8 @@ export type UserStatus = "active" | "dormant" | "suspended"; // From codersdk/insights.go export interface UserStatusChangeCount { - readonly date: string; - readonly count: number; + readonly date: string; + readonly count: number; } export const UserStatuses: UserStatus[] = ["active", "dormant", "suspended"]; From ecffc8b9dd218adbe28615df7644c9d99db231d7 Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Tue, 24 Dec 2024 17:32:20 +0000 Subject: [PATCH 14/32] update field names and fix tests --- coderd/database/dbmem/dbmem.go | 11 ++++++----- coderd/database/querier_test.go | 20 ++++++++++---------- coderd/database/queries.sql.go | 18 +++++++++--------- coderd/database/queries/insights.sql | 10 +++++----- coderd/insights.go | 4 ++-- 5 files changed, 32 insertions(+), 31 deletions(-) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 58a29158ea392..07f77ccb460cb 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -5685,17 +5685,18 @@ func (q *FakeQuerier) GetUserStatusChanges(_ context.Context, arg database.GetUs if change.ChangedAt.Before(arg.StartTime) || change.ChangedAt.After(arg.EndTime) { continue } + date := time.Date(change.ChangedAt.Year(), change.ChangedAt.Month(), change.ChangedAt.Day(), 0, 0, 0, 0, time.UTC) if !slices.ContainsFunc(result, func(r database.GetUserStatusChangesRow) bool { - return r.ChangedAt.Equal(change.ChangedAt) && r.NewStatus == change.NewStatus + return r.Status == change.NewStatus && r.Date.Equal(date) }) { result = append(result, database.GetUserStatusChangesRow{ - NewStatus: change.NewStatus, - ChangedAt: change.ChangedAt, - Count: 1, + Status: change.NewStatus, + Date: date, + Count: 1, }) } else { for i, r := range result { - if r.ChangedAt.Equal(change.ChangedAt) && r.NewStatus == change.NewStatus { + if r.Status == change.NewStatus && r.Date.Equal(date) { result[i].Count++ break } diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index 1c5d4778f6dcb..36ce5401cfc86 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -2328,7 +2328,7 @@ func TestGetUserStatusChanges(t *testing.T) { // We should have an entry for each status change require.Len(t, userStatusChanges, 5, "should have 1 user * 5 days = 5 rows") for _, row := range userStatusChanges { - require.Equal(t, row.NewStatus, tc.status, "should have the correct status") + require.Equal(t, row.Status, tc.status, "should have the correct status") } }) } @@ -2422,10 +2422,10 @@ func TestGetUserStatusChanges(t *testing.T) { // We should have an entry for each status (active, dormant, suspended) for each day require.Len(t, userStatusChanges, 5, "should have 1 user * 5 days = 5 rows") for _, row := range userStatusChanges { - if row.ChangedAt.Before(firstTransitionTime) { - require.Equal(t, row.NewStatus, tc.initialStatus, "should have the initial status") + if row.Date.Before(firstTransitionTime) { + require.Equal(t, row.Status, tc.initialStatus, "should have the initial status") } else { - require.Equal(t, row.NewStatus, tc.targetStatus, "should have the target status") + require.Equal(t, row.Status, tc.targetStatus, "should have the target status") } } }) @@ -2638,12 +2638,12 @@ func TestGetUserStatusChanges(t *testing.T) { // Expected counts before, between and after the transitions should match: for _, row := range userStatusChanges { switch { - case row.ChangedAt.Before(firstTransitionTime): - require.Equal(t, row.Count, tc.expectedCounts["initial"][row.NewStatus], "should have the correct count before the first transition") - case row.ChangedAt.Before(secondTransitionTime): - require.Equal(t, row.Count, tc.expectedCounts["between"][row.NewStatus], "should have the correct count between the transitions") - case row.ChangedAt.Before(now): - require.Equal(t, row.Count, tc.expectedCounts["final"][row.NewStatus], "should have the correct count after the second transition") + case row.Date.Before(firstTransitionTime): + require.Equal(t, row.Count, tc.expectedCounts["initial"][row.Status], "should have the correct count before the first transition") + case row.Date.Before(secondTransitionTime): + require.Equal(t, row.Count, tc.expectedCounts["between"][row.Status], "should have the correct count between the transitions") + case row.Date.Before(now): + require.Equal(t, row.Count, tc.expectedCounts["final"][row.Status], "should have the correct count after the second transition") } } }) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index d8074c57e9a17..1e7e54eae4d2d 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3096,11 +3096,11 @@ func (q *sqlQuerier) GetUserLatencyInsights(ctx context.Context, arg GetUserLate const getUserStatusChanges = `-- name: GetUserStatusChanges :many WITH dates AS ( - SELECT (generate_series( + SELECT generate_series( date_trunc('day', $1::timestamptz), date_trunc('day', $2::timestamptz), '1 day'::interval - ) AT TIME ZONE (extract(timezone FROM $1::timestamptz)::text || ' minutes'))::timestamptz AS date + )::timestamptz AS date ), latest_status_before_range AS ( -- Get the last status change for each user before our date range @@ -3148,13 +3148,13 @@ daily_counts AS ( GROUP BY d.date, asc1.new_status ) SELECT - date AS changed_at, - new_status, + date::timestamptz AS date, + new_status AS status, count FROM daily_counts WHERE count > 0 ORDER BY - new_status ASC, + status ASC, date ASC ` @@ -3164,9 +3164,9 @@ type GetUserStatusChangesParams struct { } type GetUserStatusChangesRow struct { - ChangedAt time.Time `db:"changed_at" json:"changed_at"` - NewStatus UserStatus `db:"new_status" json:"new_status"` - Count int64 `db:"count" json:"count"` + Date time.Time `db:"date" json:"date"` + Status UserStatus `db:"status" json:"status"` + Count int64 `db:"count" json:"count"` } func (q *sqlQuerier) GetUserStatusChanges(ctx context.Context, arg GetUserStatusChangesParams) ([]GetUserStatusChangesRow, error) { @@ -3178,7 +3178,7 @@ func (q *sqlQuerier) GetUserStatusChanges(ctx context.Context, arg GetUserStatus var items []GetUserStatusChangesRow for rows.Next() { var i GetUserStatusChangesRow - if err := rows.Scan(&i.ChangedAt, &i.NewStatus, &i.Count); err != nil { + if err := rows.Scan(&i.Date, &i.Status, &i.Count); err != nil { return nil, err } items = append(items, i) diff --git a/coderd/database/queries/insights.sql b/coderd/database/queries/insights.sql index 18ea61de32773..698acbfba6605 100644 --- a/coderd/database/queries/insights.sql +++ b/coderd/database/queries/insights.sql @@ -774,11 +774,11 @@ GROUP BY utp.num, utp.template_ids, utp.name, utp.type, utp.display_name, utp.de -- name: GetUserStatusChanges :many WITH dates AS ( - SELECT (generate_series( + SELECT generate_series( date_trunc('day', @start_time::timestamptz), date_trunc('day', @end_time::timestamptz), '1 day'::interval - ) AT TIME ZONE (extract(timezone FROM @start_time::timestamptz)::text || ' minutes'))::timestamptz AS date + )::timestamptz AS date ), latest_status_before_range AS ( -- Get the last status change for each user before our date range @@ -826,11 +826,11 @@ daily_counts AS ( GROUP BY d.date, asc1.new_status ) SELECT - date AS changed_at, - new_status, + date::timestamptz AS date, + new_status AS status, count FROM daily_counts WHERE count > 0 ORDER BY - new_status ASC, + status ASC, date ASC; diff --git a/coderd/insights.go b/coderd/insights.go index dfa914425f965..3cd9a364a2efe 100644 --- a/coderd/insights.go +++ b/coderd/insights.go @@ -341,12 +341,12 @@ func (api *API) insightsUserStatusCountsOverTime(rw http.ResponseWriter, r *http } for _, row := range rows { - status := codersdk.UserStatus(row.NewStatus) + status := codersdk.UserStatus(row.Status) if _, ok := resp.StatusCounts[status]; !ok { resp.StatusCounts[status] = make([]codersdk.UserStatusChangeCount, 0) } resp.StatusCounts[status] = append(resp.StatusCounts[status], codersdk.UserStatusChangeCount{ - Date: row.ChangedAt, + Date: row.Date, Count: row.Count, }) } From f3a2ce34d5fd209decc70792ae3f10d09a65eb53 Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Fri, 27 Dec 2024 12:17:23 +0000 Subject: [PATCH 15/32] exclude deleted users from the user status graph --- coderd/database/dump.sql | 27 +++++++++++++++++++ coderd/database/foreign_key_constraint.go | 1 + .../000280_user_status_changes.down.sql | 6 ++--- .../000280_user_status_changes.up.sql | 21 +++++++++++++++ coderd/database/models.go | 7 +++++ coderd/database/queries.sql.go | 8 +++++- coderd/database/queries/insights.sql | 8 +++++- coderd/database/unique_constraint.go | 1 + 8 files changed, 73 insertions(+), 6 deletions(-) diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index a03bc5dedcd0b..7812f6e8e4e5a 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -438,6 +438,17 @@ BEGIN NEW.updated_at ); END IF; + + IF OLD.deleted = FALSE AND NEW.deleted = TRUE THEN + INSERT INTO user_deleted ( + user_id, + deleted_at + ) VALUES ( + NEW.id, + NEW.updated_at + ); + END IF; + RETURN NEW; END; $$; @@ -1396,6 +1407,14 @@ CREATE VIEW template_with_names AS COMMENT ON VIEW template_with_names IS 'Joins in the display name information such as username, avatar, and organization name.'; +CREATE TABLE user_deleted ( + id uuid DEFAULT gen_random_uuid() NOT NULL, + user_id uuid NOT NULL, + deleted_at timestamp with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL +); + +COMMENT ON TABLE user_deleted IS 'Tracks when users were deleted'; + CREATE TABLE user_links ( user_id uuid NOT NULL, login_type login_type NOT NULL, @@ -2008,6 +2027,9 @@ ALTER TABLE ONLY template_versions ALTER TABLE ONLY templates ADD CONSTRAINT templates_pkey PRIMARY KEY (id); +ALTER TABLE ONLY user_deleted + ADD CONSTRAINT user_deleted_pkey PRIMARY KEY (id); + ALTER TABLE ONLY user_links ADD CONSTRAINT user_links_pkey PRIMARY KEY (user_id, login_type); @@ -2124,6 +2146,8 @@ CREATE INDEX idx_tailnet_tunnels_dst_id ON tailnet_tunnels USING hash (dst_id); CREATE INDEX idx_tailnet_tunnels_src_id ON tailnet_tunnels USING hash (src_id); +CREATE INDEX idx_user_deleted_deleted_at ON user_deleted USING btree (deleted_at); + CREATE INDEX idx_user_status_changes_changed_at ON user_status_changes USING btree (changed_at); CREATE UNIQUE INDEX idx_users_email ON users USING btree (email) WHERE (deleted = false); @@ -2393,6 +2417,9 @@ ALTER TABLE ONLY templates ALTER TABLE ONLY templates ADD CONSTRAINT templates_organization_id_fkey FOREIGN KEY (organization_id) REFERENCES organizations(id) ON DELETE CASCADE; +ALTER TABLE ONLY user_deleted + ADD CONSTRAINT user_deleted_user_id_fkey FOREIGN KEY (user_id) REFERENCES users(id); + ALTER TABLE ONLY user_links ADD CONSTRAINT user_links_oauth_access_token_key_id_fkey FOREIGN KEY (oauth_access_token_key_id) REFERENCES dbcrypt_keys(active_key_digest); diff --git a/coderd/database/foreign_key_constraint.go b/coderd/database/foreign_key_constraint.go index 18c82b83750fa..52f98a679a71b 100644 --- a/coderd/database/foreign_key_constraint.go +++ b/coderd/database/foreign_key_constraint.go @@ -47,6 +47,7 @@ const ( ForeignKeyTemplateVersionsTemplateID ForeignKeyConstraint = "template_versions_template_id_fkey" // ALTER TABLE ONLY template_versions ADD CONSTRAINT template_versions_template_id_fkey FOREIGN KEY (template_id) REFERENCES templates(id) ON DELETE CASCADE; ForeignKeyTemplatesCreatedBy ForeignKeyConstraint = "templates_created_by_fkey" // ALTER TABLE ONLY templates ADD CONSTRAINT templates_created_by_fkey FOREIGN KEY (created_by) REFERENCES users(id) ON DELETE RESTRICT; ForeignKeyTemplatesOrganizationID ForeignKeyConstraint = "templates_organization_id_fkey" // ALTER TABLE ONLY templates ADD CONSTRAINT templates_organization_id_fkey FOREIGN KEY (organization_id) REFERENCES organizations(id) ON DELETE CASCADE; + ForeignKeyUserDeletedUserID ForeignKeyConstraint = "user_deleted_user_id_fkey" // ALTER TABLE ONLY user_deleted ADD CONSTRAINT user_deleted_user_id_fkey FOREIGN KEY (user_id) REFERENCES users(id); ForeignKeyUserLinksOauthAccessTokenKeyID ForeignKeyConstraint = "user_links_oauth_access_token_key_id_fkey" // ALTER TABLE ONLY user_links ADD CONSTRAINT user_links_oauth_access_token_key_id_fkey FOREIGN KEY (oauth_access_token_key_id) REFERENCES dbcrypt_keys(active_key_digest); ForeignKeyUserLinksOauthRefreshTokenKeyID ForeignKeyConstraint = "user_links_oauth_refresh_token_key_id_fkey" // ALTER TABLE ONLY user_links ADD CONSTRAINT user_links_oauth_refresh_token_key_id_fkey FOREIGN KEY (oauth_refresh_token_key_id) REFERENCES dbcrypt_keys(active_key_digest); ForeignKeyUserLinksUserID ForeignKeyConstraint = "user_links_user_id_fkey" // ALTER TABLE ONLY user_links ADD CONSTRAINT user_links_user_id_fkey FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; diff --git a/coderd/database/migrations/000280_user_status_changes.down.sql b/coderd/database/migrations/000280_user_status_changes.down.sql index 30514ef467ebe..fbe85a6be0fe5 100644 --- a/coderd/database/migrations/000280_user_status_changes.down.sql +++ b/coderd/database/migrations/000280_user_status_changes.down.sql @@ -1,11 +1,9 @@ --- Drop the trigger first DROP TRIGGER IF EXISTS user_status_change_trigger ON users; --- Drop the trigger function DROP FUNCTION IF EXISTS record_user_status_change(); --- Drop the indexes DROP INDEX IF EXISTS idx_user_status_changes_changed_at; +DROP INDEX IF EXISTS idx_user_deleted_deleted_at; --- Drop the table DROP TABLE IF EXISTS user_status_changes; +DROP TABLE IF EXISTS user_deleted; diff --git a/coderd/database/migrations/000280_user_status_changes.up.sql b/coderd/database/migrations/000280_user_status_changes.up.sql index f16164862b3e5..04d8472e55460 100644 --- a/coderd/database/migrations/000280_user_status_changes.up.sql +++ b/coderd/database/migrations/000280_user_status_changes.up.sql @@ -21,6 +21,16 @@ SELECT FROM users WHERE NOT deleted; +CREATE TABLE user_deleted ( + id uuid PRIMARY KEY DEFAULT gen_random_uuid(), + user_id uuid NOT NULL REFERENCES users(id), + deleted_at timestamptz NOT NULL DEFAULT CURRENT_TIMESTAMP +); + +COMMENT ON TABLE user_deleted IS 'Tracks when users were deleted'; + +CREATE INDEX idx_user_deleted_deleted_at ON user_deleted(deleted_at); + CREATE OR REPLACE FUNCTION record_user_status_change() RETURNS trigger AS $$ BEGIN IF TG_OP = 'INSERT' OR OLD.status IS DISTINCT FROM NEW.status THEN @@ -34,6 +44,17 @@ BEGIN NEW.updated_at ); END IF; + + IF OLD.deleted = FALSE AND NEW.deleted = TRUE THEN + INSERT INTO user_deleted ( + user_id, + deleted_at + ) VALUES ( + NEW.id, + NEW.updated_at + ); + END IF; + RETURN NEW; END; $$ LANGUAGE plpgsql; diff --git a/coderd/database/models.go b/coderd/database/models.go index 886626a58b3f3..35484c7856e14 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -2953,6 +2953,13 @@ type User struct { OneTimePasscodeExpiresAt sql.NullTime `db:"one_time_passcode_expires_at" json:"one_time_passcode_expires_at"` } +// Tracks when users were deleted +type UserDeleted struct { + ID uuid.UUID `db:"id" json:"id"` + UserID uuid.UUID `db:"user_id" json:"user_id"` + DeletedAt time.Time `db:"deleted_at" json:"deleted_at"` +} + type UserLink struct { UserID uuid.UUID `db:"user_id" json:"user_id"` LoginType LoginType `db:"login_type" json:"login_type"` diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 1e7e54eae4d2d..c70ffc11a9193 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3134,7 +3134,7 @@ daily_counts AS ( d.date, asc1.new_status, -- For each date and status, count users whose most recent status change - -- (up to that date) matches this status + -- (up to that date) matches this status AND who weren't deleted by that date COUNT(*) FILTER ( WHERE asc1.changed_at = ( SELECT MAX(changed_at) @@ -3142,6 +3142,12 @@ daily_counts AS ( WHERE asc2.user_id = asc1.user_id AND asc2.changed_at <= d.date ) + AND NOT EXISTS ( + SELECT 1 + FROM user_deleted ud + WHERE ud.user_id = asc1.user_id + AND ud.deleted_at <= d.date + ) )::bigint AS count FROM dates d CROSS JOIN all_status_changes asc1 diff --git a/coderd/database/queries/insights.sql b/coderd/database/queries/insights.sql index 698acbfba6605..04e74bf7f166e 100644 --- a/coderd/database/queries/insights.sql +++ b/coderd/database/queries/insights.sql @@ -812,7 +812,7 @@ daily_counts AS ( d.date, asc1.new_status, -- For each date and status, count users whose most recent status change - -- (up to that date) matches this status + -- (up to that date) matches this status AND who weren't deleted by that date COUNT(*) FILTER ( WHERE asc1.changed_at = ( SELECT MAX(changed_at) @@ -820,6 +820,12 @@ daily_counts AS ( WHERE asc2.user_id = asc1.user_id AND asc2.changed_at <= d.date ) + AND NOT EXISTS ( + SELECT 1 + FROM user_deleted ud + WHERE ud.user_id = asc1.user_id + AND ud.deleted_at <= d.date + ) )::bigint AS count FROM dates d CROSS JOIN all_status_changes asc1 diff --git a/coderd/database/unique_constraint.go b/coderd/database/unique_constraint.go index b59cb0cbc8091..f253aa98ec266 100644 --- a/coderd/database/unique_constraint.go +++ b/coderd/database/unique_constraint.go @@ -62,6 +62,7 @@ const ( UniqueTemplateVersionsPkey UniqueConstraint = "template_versions_pkey" // ALTER TABLE ONLY template_versions ADD CONSTRAINT template_versions_pkey PRIMARY KEY (id); UniqueTemplateVersionsTemplateIDNameKey UniqueConstraint = "template_versions_template_id_name_key" // ALTER TABLE ONLY template_versions ADD CONSTRAINT template_versions_template_id_name_key UNIQUE (template_id, name); UniqueTemplatesPkey UniqueConstraint = "templates_pkey" // ALTER TABLE ONLY templates ADD CONSTRAINT templates_pkey PRIMARY KEY (id); + UniqueUserDeletedPkey UniqueConstraint = "user_deleted_pkey" // ALTER TABLE ONLY user_deleted ADD CONSTRAINT user_deleted_pkey PRIMARY KEY (id); UniqueUserLinksPkey UniqueConstraint = "user_links_pkey" // ALTER TABLE ONLY user_links ADD CONSTRAINT user_links_pkey PRIMARY KEY (user_id, login_type); UniqueUserStatusChangesPkey UniqueConstraint = "user_status_changes_pkey" // ALTER TABLE ONLY user_status_changes ADD CONSTRAINT user_status_changes_pkey PRIMARY KEY (id); UniqueUsersPkey UniqueConstraint = "users_pkey" // ALTER TABLE ONLY users ADD CONSTRAINT users_pkey PRIMARY KEY (id); From 5067a637482d51d9265d16470f90ee0fbe8826ea Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Thu, 2 Jan 2025 17:18:42 +0000 Subject: [PATCH 16/32] GetUserStatusChanges now passes all querier tests --- Makefile | 5 +- coderd/database/querier_test.go | 803 ++++++++++++++++----------- coderd/database/queries.sql.go | 113 ++-- coderd/database/queries/insights.sql | 114 ++-- 4 files changed, 601 insertions(+), 434 deletions(-) diff --git a/Makefile b/Makefile index 2cd40a7dabfa3..423402260c26b 100644 --- a/Makefile +++ b/Makefile @@ -521,6 +521,7 @@ lint/markdown: node_modules/.installed # All files generated by the database should be added here, and this can be used # as a target for jobs that need to run after the database is generated. DB_GEN_FILES := \ + coderd/database/dump.sql \ coderd/database/querier.go \ coderd/database/unique_constraint.go \ coderd/database/dbmem/dbmem.go \ @@ -540,8 +541,6 @@ GEN_FILES := \ provisionersdk/proto/provisioner.pb.go \ provisionerd/proto/provisionerd.pb.go \ vpn/vpn.pb.go \ - coderd/database/dump.sql \ - $(DB_GEN_FILES) \ site/src/api/typesGenerated.ts \ coderd/rbac/object_gen.go \ codersdk/rbacresources_gen.go \ @@ -559,7 +558,7 @@ GEN_FILES := \ coderd/database/pubsub/psmock/psmock.go # all gen targets should be added here and to gen/mark-fresh -gen: $(GEN_FILES) +gen: gen/db $(GEN_FILES) .PHONY: gen gen/db: $(DB_GEN_FILES) diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index 36ce5401cfc86..6ace33f74a96c 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -7,6 +7,7 @@ import ( "database/sql" "encoding/json" "fmt" + "maps" "sort" "testing" "time" @@ -2258,397 +2259,525 @@ func TestGroupRemovalTrigger(t *testing.T) { func TestGetUserStatusChanges(t *testing.T) { t.Parallel() - now := dbtime.Now() - createdAt := now.Add(-5 * 24 * time.Hour) // 5 days ago - firstTransitionTime := createdAt.Add(2 * 24 * time.Hour) // 3 days ago - secondTransitionTime := firstTransitionTime.Add(2 * 24 * time.Hour) // 1 days ago - t.Run("No Users", func(t *testing.T) { - t.Parallel() - db, _ := dbtestutil.NewDB(t) - ctx := testutil.Context(t, testutil.WaitShort) - - end := dbtime.Now() - start := end.Add(-30 * 24 * time.Hour) + if !dbtestutil.WillUsePostgres() { + t.SkipNow() + } - counts, err := db.GetUserStatusChanges(ctx, database.GetUserStatusChangesParams{ - StartTime: start, - EndTime: end, - }) - require.NoError(t, err) - require.Empty(t, counts, "should return no results when there are no users") - }) + timezones := []string{ + "Canada/Newfoundland", + "Africa/Johannesburg", + "America/New_York", + "Europe/London", + "Asia/Tokyo", + "Australia/Sydney", + } - t.Run("Single User/Single State", func(t *testing.T) { - t.Parallel() + for _, tz := range timezones { + tz := tz + t.Run(tz, func(t *testing.T) { + t.Parallel() - testCases := []struct { - name string - status database.UserStatus - }{ - { - name: "Active Only", - status: database.UserStatusActive, - }, - { - name: "Dormant Only", - status: database.UserStatusDormant, - }, - { - name: "Suspended Only", - status: database.UserStatusSuspended, - }, - // { - // name: "Deleted Only", - // status: database.UserStatusDeleted, - // }, - } + location, err := time.LoadLocation(tz) + if err != nil { + t.Fatalf("failed to load location: %v", err) + } + today := dbtime.Now().In(location) + createdAt := today.Add(-5 * 24 * time.Hour) + firstTransitionTime := createdAt.Add(2 * 24 * time.Hour) + secondTransitionTime := firstTransitionTime.Add(2 * 24 * time.Hour) - for _, tc := range testCases { - tc := tc - t.Run(tc.name, func(t *testing.T) { + t.Run("No Users", func(t *testing.T) { t.Parallel() db, _ := dbtestutil.NewDB(t) ctx := testutil.Context(t, testutil.WaitShort) - // Create a user that's been in the specified status for the past 30 days - dbgen.User(t, db, database.User{ - Status: tc.status, - CreatedAt: createdAt, - UpdatedAt: createdAt, - }) + end := dbtime.Now() + start := end.Add(-30 * 24 * time.Hour) - // Query for the last 30 days - userStatusChanges, err := db.GetUserStatusChanges(ctx, database.GetUserStatusChangesParams{ - StartTime: createdAt, - EndTime: now, + counts, err := db.GetUserStatusChanges(ctx, database.GetUserStatusChangesParams{ + StartTime: start, + EndTime: end, }) require.NoError(t, err) - require.NotEmpty(t, userStatusChanges, "should return results") - - // We should have an entry for each status change - require.Len(t, userStatusChanges, 5, "should have 1 user * 5 days = 5 rows") - for _, row := range userStatusChanges { - require.Equal(t, row.Status, tc.status, "should have the correct status") - } + require.Empty(t, counts, "should return no results when there are no users") }) - } - }) - - t.Run("Single Transition", func(t *testing.T) { - t.Parallel() - - testCases := []struct { - name string - initialStatus database.UserStatus - targetStatus database.UserStatus - }{ - { - name: "Active to Dormant", - initialStatus: database.UserStatusActive, - targetStatus: database.UserStatusDormant, - }, - { - name: "Active to Suspended", - initialStatus: database.UserStatusActive, - targetStatus: database.UserStatusSuspended, - }, - // { - // name: "Active to Deleted", - // initialStatus: database.UserStatusActive, - // targetStatus: database.UserStatusDeleted, - // }, - { - name: "Dormant to Active", - initialStatus: database.UserStatusDormant, - targetStatus: database.UserStatusActive, - }, - { - name: "Dormant to Suspended", - initialStatus: database.UserStatusDormant, - targetStatus: database.UserStatusSuspended, - }, - // { - // name: "Dormant to Deleted", - // initialStatus: database.UserStatusDormant, - // targetStatus: database.UserStatusDeleted, - // }, - { - name: "Suspended to Active", - initialStatus: database.UserStatusSuspended, - targetStatus: database.UserStatusActive, - }, - { - name: "Suspended to Dormant", - initialStatus: database.UserStatusSuspended, - targetStatus: database.UserStatusDormant, - }, - // { - // name: "Suspended to Deleted", - // initialStatus: database.UserStatusSuspended, - // targetStatus: database.UserStatusDeleted, - // }, - } - for _, tc := range testCases { - tc := tc - t.Run(tc.name, func(t *testing.T) { + t.Run("One User/Creation Only", func(t *testing.T) { t.Parallel() - db, _ := dbtestutil.NewDB(t) - ctx := testutil.Context(t, testutil.WaitShort) - // Create a user that starts with initial status - user := dbgen.User(t, db, database.User{ - Status: tc.initialStatus, - CreatedAt: createdAt, - UpdatedAt: createdAt, - }) - - // After 2 days, change status to target status - user, err := db.UpdateUserStatus(ctx, database.UpdateUserStatusParams{ - ID: user.ID, - Status: tc.targetStatus, - UpdatedAt: firstTransitionTime, - }) - require.NoError(t, err) + testCases := []struct { + name string + status database.UserStatus + }{ + { + name: "Active Only", + status: database.UserStatusActive, + }, + { + name: "Dormant Only", + status: database.UserStatusDormant, + }, + { + name: "Suspended Only", + status: database.UserStatusSuspended, + }, + // { + // name: "Deleted Only", + // status: database.UserStatusDeleted, + // }, + } - // Query for the last 5 days - userStatusChanges, err := db.GetUserStatusChanges(ctx, database.GetUserStatusChangesParams{ - StartTime: createdAt, - EndTime: now, - }) - require.NoError(t, err) - require.NotEmpty(t, userStatusChanges, "should return results") - - // We should have an entry for each status (active, dormant, suspended) for each day - require.Len(t, userStatusChanges, 5, "should have 1 user * 5 days = 5 rows") - for _, row := range userStatusChanges { - if row.Date.Before(firstTransitionTime) { - require.Equal(t, row.Status, tc.initialStatus, "should have the initial status") - } else { - require.Equal(t, row.Status, tc.targetStatus, "should have the target status") - } + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + db, _ := dbtestutil.NewDB(t) + ctx := testutil.Context(t, testutil.WaitShort) + + // Create a user that's been in the specified status for the past 30 days + dbgen.User(t, db, database.User{ + Status: tc.status, + CreatedAt: createdAt, + UpdatedAt: createdAt, + }) + + // Query for the last 30 days + userStatusChanges, err := db.GetUserStatusChanges(ctx, database.GetUserStatusChangesParams{ + StartTime: createdAt, + EndTime: today, + }) + require.NoError(t, err) + require.NotEmpty(t, userStatusChanges, "should return results") + + require.Len(t, userStatusChanges, 2, "should have 1 entry per status change plus and 1 entry for the end of the range = 2 entries") + + require.Equal(t, userStatusChanges[0].Status, tc.status, "should have the correct status") + require.Equal(t, userStatusChanges[0].Count, int64(1), "should have 1 user") + require.True(t, userStatusChanges[0].Date.Equal(createdAt), "should have the correct date") + + require.Equal(t, userStatusChanges[1].Status, tc.status, "should have the correct status") + require.Equal(t, userStatusChanges[1].Count, int64(1), "should have 1 user") + require.True(t, userStatusChanges[1].Date.Equal(today), "should have the correct date") + }) } }) - } - }) - t.Run("Two Users Transitioning", func(t *testing.T) { - t.Parallel() - - type transition struct { - from database.UserStatus - to database.UserStatus - } - - type testCase struct { - name string - user1Transition transition - user2Transition transition - expectedCounts map[string]map[database.UserStatus]int64 - } + t.Run("One User/One Transition", func(t *testing.T) { + t.Parallel() - testCases := []testCase{ - { - name: "Active->Dormant and Dormant->Suspended", - user1Transition: transition{ - from: database.UserStatusActive, - to: database.UserStatusDormant, - }, - user2Transition: transition{ - from: database.UserStatusDormant, - to: database.UserStatusSuspended, - }, - expectedCounts: map[string]map[database.UserStatus]int64{ - "initial": { - database.UserStatusActive: 1, - database.UserStatusDormant: 1, - database.UserStatusSuspended: 0, - }, - "between": { - database.UserStatusActive: 0, - database.UserStatusDormant: 2, - database.UserStatusSuspended: 0, - }, - "final": { - database.UserStatusActive: 0, - database.UserStatusDormant: 1, - database.UserStatusSuspended: 1, - }, - }, - }, - { - name: "Suspended->Active and Active->Dormant", - user1Transition: transition{ - from: database.UserStatusSuspended, - to: database.UserStatusActive, - }, - user2Transition: transition{ - from: database.UserStatusActive, - to: database.UserStatusDormant, - }, - expectedCounts: map[string]map[database.UserStatus]int64{ - "initial": { - database.UserStatusActive: 1, - database.UserStatusDormant: 0, - database.UserStatusSuspended: 1, - }, - "between": { - database.UserStatusActive: 2, - database.UserStatusDormant: 0, - database.UserStatusSuspended: 0, + testCases := []struct { + name string + initialStatus database.UserStatus + targetStatus database.UserStatus + expectedCounts map[time.Time]map[database.UserStatus]int64 + }{ + { + name: "Active to Dormant", + initialStatus: database.UserStatusActive, + targetStatus: database.UserStatusDormant, + expectedCounts: map[time.Time]map[database.UserStatus]int64{ + createdAt: { + database.UserStatusActive: 1, + database.UserStatusDormant: 0, + }, + firstTransitionTime: { + database.UserStatusDormant: 1, + database.UserStatusActive: 0, + }, + today: { + database.UserStatusDormant: 1, + database.UserStatusActive: 0, + }, + }, }, - "final": { - database.UserStatusActive: 1, - database.UserStatusDormant: 1, - database.UserStatusSuspended: 0, + { + name: "Active to Suspended", + initialStatus: database.UserStatusActive, + targetStatus: database.UserStatusSuspended, + expectedCounts: map[time.Time]map[database.UserStatus]int64{ + createdAt: { + database.UserStatusActive: 1, + database.UserStatusSuspended: 0, + }, + firstTransitionTime: { + database.UserStatusSuspended: 1, + database.UserStatusActive: 0, + }, + today: { + database.UserStatusSuspended: 1, + database.UserStatusActive: 0, + }, + }, }, - }, - }, - { - name: "Dormant->Active and Suspended->Dormant", - user1Transition: transition{ - from: database.UserStatusDormant, - to: database.UserStatusActive, - }, - user2Transition: transition{ - from: database.UserStatusSuspended, - to: database.UserStatusDormant, - }, - expectedCounts: map[string]map[database.UserStatus]int64{ - "initial": { - database.UserStatusActive: 0, - database.UserStatusDormant: 1, - database.UserStatusSuspended: 1, + { + name: "Dormant to Active", + initialStatus: database.UserStatusDormant, + targetStatus: database.UserStatusActive, + expectedCounts: map[time.Time]map[database.UserStatus]int64{ + createdAt: { + database.UserStatusDormant: 1, + database.UserStatusActive: 0, + }, + firstTransitionTime: { + database.UserStatusActive: 1, + database.UserStatusDormant: 0, + }, + today: { + database.UserStatusActive: 1, + database.UserStatusDormant: 0, + }, + }, }, - "between": { - database.UserStatusActive: 1, - database.UserStatusDormant: 0, - database.UserStatusSuspended: 1, + { + name: "Dormant to Suspended", + initialStatus: database.UserStatusDormant, + targetStatus: database.UserStatusSuspended, + expectedCounts: map[time.Time]map[database.UserStatus]int64{ + createdAt: { + database.UserStatusDormant: 1, + database.UserStatusSuspended: 0, + }, + firstTransitionTime: { + database.UserStatusSuspended: 1, + database.UserStatusDormant: 0, + }, + today: { + database.UserStatusSuspended: 1, + database.UserStatusDormant: 0, + }, + }, }, - "final": { - database.UserStatusActive: 1, - database.UserStatusDormant: 1, - database.UserStatusSuspended: 0, + { + name: "Suspended to Active", + initialStatus: database.UserStatusSuspended, + targetStatus: database.UserStatusActive, + expectedCounts: map[time.Time]map[database.UserStatus]int64{ + createdAt: { + database.UserStatusSuspended: 1, + database.UserStatusActive: 0, + }, + firstTransitionTime: { + database.UserStatusActive: 1, + database.UserStatusSuspended: 0, + }, + today: { + database.UserStatusActive: 1, + database.UserStatusSuspended: 0, + }, + }, }, - }, - }, - { - name: "Active->Suspended and Suspended->Active", - user1Transition: transition{ - from: database.UserStatusActive, - to: database.UserStatusSuspended, - }, - user2Transition: transition{ - from: database.UserStatusSuspended, - to: database.UserStatusActive, - }, - expectedCounts: map[string]map[database.UserStatus]int64{ - "initial": { - database.UserStatusActive: 1, - database.UserStatusDormant: 0, - database.UserStatusSuspended: 1, + { + name: "Suspended to Dormant", + initialStatus: database.UserStatusSuspended, + targetStatus: database.UserStatusDormant, + expectedCounts: map[time.Time]map[database.UserStatus]int64{ + createdAt: { + database.UserStatusSuspended: 1, + database.UserStatusDormant: 0, + }, + firstTransitionTime: { + database.UserStatusDormant: 1, + database.UserStatusSuspended: 0, + }, + today: { + database.UserStatusDormant: 1, + database.UserStatusSuspended: 0, + }, + }, }, - "between": { - database.UserStatusActive: 0, - database.UserStatusDormant: 0, - database.UserStatusSuspended: 2, + } + + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + db, _ := dbtestutil.NewDB(t) + ctx := testutil.Context(t, testutil.WaitShort) + + // Create a user that starts with initial status + user := dbgen.User(t, db, database.User{ + Status: tc.initialStatus, + CreatedAt: createdAt, + UpdatedAt: createdAt, + }) + + // After 2 days, change status to target status + user, err := db.UpdateUserStatus(ctx, database.UpdateUserStatusParams{ + ID: user.ID, + Status: tc.targetStatus, + UpdatedAt: firstTransitionTime, + }) + require.NoError(t, err) + + // Query for the last 5 days + userStatusChanges, err := db.GetUserStatusChanges(ctx, database.GetUserStatusChangesParams{ + StartTime: createdAt, + EndTime: today, + }) + require.NoError(t, err) + require.NotEmpty(t, userStatusChanges, "should return results") + + gotCounts := map[time.Time]map[database.UserStatus]int64{} + for _, row := range userStatusChanges { + gotDateInLocation := row.Date.In(location) + if _, ok := gotCounts[gotDateInLocation]; !ok { + gotCounts[gotDateInLocation] = map[database.UserStatus]int64{} + } + if _, ok := gotCounts[gotDateInLocation][row.Status]; !ok { + gotCounts[gotDateInLocation][row.Status] = 0 + } + gotCounts[gotDateInLocation][row.Status] += row.Count + } + require.Equal(t, tc.expectedCounts, gotCounts) + }) + } + }) + + t.Run("Two Users/One Transition", func(t *testing.T) { + t.Parallel() + + type transition struct { + from database.UserStatus + to database.UserStatus + } + + type testCase struct { + name string + user1Transition transition + user2Transition transition + } + + testCases := []testCase{ + { + name: "Active->Dormant and Dormant->Suspended", + user1Transition: transition{ + from: database.UserStatusActive, + to: database.UserStatusDormant, + }, + user2Transition: transition{ + from: database.UserStatusDormant, + to: database.UserStatusSuspended, + }, }, - "final": { - database.UserStatusActive: 1, - database.UserStatusDormant: 0, - database.UserStatusSuspended: 1, + { + name: "Suspended->Active and Active->Dormant", + user1Transition: transition{ + from: database.UserStatusSuspended, + to: database.UserStatusActive, + }, + user2Transition: transition{ + from: database.UserStatusActive, + to: database.UserStatusDormant, + }, }, - }, - }, - { - name: "Dormant->Suspended and Dormant->Active", - user1Transition: transition{ - from: database.UserStatusDormant, - to: database.UserStatusSuspended, - }, - user2Transition: transition{ - from: database.UserStatusDormant, - to: database.UserStatusActive, - }, - expectedCounts: map[string]map[database.UserStatus]int64{ - "initial": { - database.UserStatusActive: 0, - database.UserStatusDormant: 2, - database.UserStatusSuspended: 0, + { + name: "Dormant->Active and Suspended->Dormant", + user1Transition: transition{ + from: database.UserStatusDormant, + to: database.UserStatusActive, + }, + user2Transition: transition{ + from: database.UserStatusSuspended, + to: database.UserStatusDormant, + }, }, - "between": { - database.UserStatusActive: 0, - database.UserStatusDormant: 1, - database.UserStatusSuspended: 1, + { + name: "Active->Suspended and Suspended->Active", + user1Transition: transition{ + from: database.UserStatusActive, + to: database.UserStatusSuspended, + }, + user2Transition: transition{ + from: database.UserStatusSuspended, + to: database.UserStatusActive, + }, }, - "final": { - database.UserStatusActive: 1, - database.UserStatusDormant: 0, - database.UserStatusSuspended: 1, + { + name: "Dormant->Suspended and Dormant->Active", + user1Transition: transition{ + from: database.UserStatusDormant, + to: database.UserStatusSuspended, + }, + user2Transition: transition{ + from: database.UserStatusDormant, + to: database.UserStatusActive, + }, }, - }, - }, - } + } - for _, tc := range testCases { - tc := tc - t.Run(tc.name, func(t *testing.T) { + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + db, _ := dbtestutil.NewDB(t) + ctx := testutil.Context(t, testutil.WaitShort) + + user1 := dbgen.User(t, db, database.User{ + Status: tc.user1Transition.from, + CreatedAt: createdAt, + UpdatedAt: createdAt, + }) + user2 := dbgen.User(t, db, database.User{ + Status: tc.user2Transition.from, + CreatedAt: createdAt, + UpdatedAt: createdAt, + }) + + // First transition at 2 days + user1, err := db.UpdateUserStatus(ctx, database.UpdateUserStatusParams{ + ID: user1.ID, + Status: tc.user1Transition.to, + UpdatedAt: firstTransitionTime, + }) + require.NoError(t, err) + + // Second transition at 4 days + user2, err = db.UpdateUserStatus(ctx, database.UpdateUserStatusParams{ + ID: user2.ID, + Status: tc.user2Transition.to, + UpdatedAt: secondTransitionTime, + }) + require.NoError(t, err) + + userStatusChanges, err := db.GetUserStatusChanges(ctx, database.GetUserStatusChangesParams{ + StartTime: createdAt, + EndTime: today, + }) + require.NoError(t, err) + require.NotEmpty(t, userStatusChanges) + gotCounts := map[time.Time]map[database.UserStatus]int64{ + createdAt.In(location): {}, + firstTransitionTime.In(location): {}, + secondTransitionTime.In(location): {}, + today.In(location): {}, + } + for _, row := range userStatusChanges { + dateInLocation := row.Date.In(location) + switch { + case dateInLocation.Equal(createdAt.In(location)): + gotCounts[createdAt][row.Status] = row.Count + case dateInLocation.Equal(firstTransitionTime.In(location)): + gotCounts[firstTransitionTime][row.Status] = row.Count + case dateInLocation.Equal(secondTransitionTime.In(location)): + gotCounts[secondTransitionTime][row.Status] = row.Count + case dateInLocation.Equal(today.In(location)): + gotCounts[today][row.Status] = row.Count + default: + t.Fatalf("unexpected date %s", row.Date) + } + } + + expectedCounts := map[time.Time]map[database.UserStatus]int64{} + for _, status := range []database.UserStatus{ + tc.user1Transition.from, + tc.user1Transition.to, + tc.user2Transition.from, + tc.user2Transition.to, + } { + if _, ok := expectedCounts[createdAt]; !ok { + expectedCounts[createdAt] = map[database.UserStatus]int64{} + } + expectedCounts[createdAt][status] = 0 + } + + expectedCounts[createdAt][tc.user1Transition.from]++ + expectedCounts[createdAt][tc.user2Transition.from]++ + + expectedCounts[firstTransitionTime] = map[database.UserStatus]int64{} + maps.Copy(expectedCounts[firstTransitionTime], expectedCounts[createdAt]) + expectedCounts[firstTransitionTime][tc.user1Transition.from]-- + expectedCounts[firstTransitionTime][tc.user1Transition.to]++ + + expectedCounts[secondTransitionTime] = map[database.UserStatus]int64{} + maps.Copy(expectedCounts[secondTransitionTime], expectedCounts[firstTransitionTime]) + expectedCounts[secondTransitionTime][tc.user2Transition.from]-- + expectedCounts[secondTransitionTime][tc.user2Transition.to]++ + + expectedCounts[today] = map[database.UserStatus]int64{} + maps.Copy(expectedCounts[today], expectedCounts[secondTransitionTime]) + + require.Equal(t, expectedCounts[createdAt], gotCounts[createdAt]) + require.Equal(t, expectedCounts[firstTransitionTime], gotCounts[firstTransitionTime]) + require.Equal(t, expectedCounts[secondTransitionTime], gotCounts[secondTransitionTime]) + require.Equal(t, expectedCounts[today], gotCounts[today]) + }) + } + }) + + t.Run("User precedes and survives query range", func(t *testing.T) { t.Parallel() db, _ := dbtestutil.NewDB(t) ctx := testutil.Context(t, testutil.WaitShort) - now := dbtime.Now() - createdAt := now.Add(-5 * 24 * time.Hour) // 5 days ago - - user1 := dbgen.User(t, db, database.User{ - Status: tc.user1Transition.from, + _ = dbgen.User(t, db, database.User{ + Status: database.UserStatusActive, CreatedAt: createdAt, UpdatedAt: createdAt, }) - user2 := dbgen.User(t, db, database.User{ - Status: tc.user2Transition.from, + + userStatusChanges, err := db.GetUserStatusChanges(ctx, database.GetUserStatusChangesParams{ + StartTime: createdAt.Add(time.Hour * 24), + EndTime: today, + }) + require.NoError(t, err) + + require.Len(t, userStatusChanges, 2) + require.Equal(t, userStatusChanges[0].Count, int64(1)) + require.Equal(t, userStatusChanges[0].Status, database.UserStatusActive) + require.Equal(t, userStatusChanges[1].Count, int64(1)) + require.Equal(t, userStatusChanges[1].Status, database.UserStatusActive) + }) + + t.Run("User deleted before query range", func(t *testing.T) { + t.Parallel() + db, _ := dbtestutil.NewDB(t) + ctx := testutil.Context(t, testutil.WaitShort) + + user := dbgen.User(t, db, database.User{ + Status: database.UserStatusActive, CreatedAt: createdAt, UpdatedAt: createdAt, }) - // First transition at 2 days - user1, err := db.UpdateUserStatus(ctx, database.UpdateUserStatusParams{ - ID: user1.ID, - Status: tc.user1Transition.to, - UpdatedAt: firstTransitionTime, + err = db.UpdateUserDeletedByID(ctx, user.ID) + require.NoError(t, err) + + userStatusChanges, err := db.GetUserStatusChanges(ctx, database.GetUserStatusChangesParams{ + StartTime: today.Add(time.Hour * 24), + EndTime: today.Add(time.Hour * 48), }) require.NoError(t, err) + require.Empty(t, userStatusChanges) + }) + + t.Run("User deleted during query range", func(t *testing.T) { + t.Parallel() + db, _ := dbtestutil.NewDB(t) + ctx := testutil.Context(t, testutil.WaitShort) - // Second transition at 4 days - user2, err = db.UpdateUserStatus(ctx, database.UpdateUserStatusParams{ - ID: user2.ID, - Status: tc.user2Transition.to, - UpdatedAt: secondTransitionTime, + user := dbgen.User(t, db, database.User{ + Status: database.UserStatusActive, + CreatedAt: createdAt, + UpdatedAt: createdAt, }) + + err := db.UpdateUserDeletedByID(ctx, user.ID) require.NoError(t, err) - // Query for the last 5 days userStatusChanges, err := db.GetUserStatusChanges(ctx, database.GetUserStatusChangesParams{ StartTime: createdAt, - EndTime: now, + EndTime: today.Add(time.Hour * 24), }) require.NoError(t, err) - require.NotEmpty(t, userStatusChanges) - - // Expected counts before, between and after the transitions should match: - for _, row := range userStatusChanges { - switch { - case row.Date.Before(firstTransitionTime): - require.Equal(t, row.Count, tc.expectedCounts["initial"][row.Status], "should have the correct count before the first transition") - case row.Date.Before(secondTransitionTime): - require.Equal(t, row.Count, tc.expectedCounts["between"][row.Status], "should have the correct count between the transitions") - case row.Date.Before(now): - require.Equal(t, row.Count, tc.expectedCounts["final"][row.Status], "should have the correct count after the second transition") - } - } + require.Equal(t, userStatusChanges[0].Count, int64(1)) + require.Equal(t, userStatusChanges[0].Status, database.UserStatusActive) + require.Equal(t, userStatusChanges[1].Count, int64(0)) + require.Equal(t, userStatusChanges[1].Status, database.UserStatusActive) + require.Equal(t, userStatusChanges[2].Count, int64(0)) + require.Equal(t, userStatusChanges[2].Status, database.UserStatusActive) }) - } - }) + }) + } } func requireUsersMatch(t testing.TB, expected []database.User, found []database.GetUsersRow, msg string) { diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index c70ffc11a9193..8d271b8f77293 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3096,24 +3096,49 @@ func (q *sqlQuerier) GetUserLatencyInsights(ctx context.Context, arg GetUserLate const getUserStatusChanges = `-- name: GetUserStatusChanges :many WITH dates AS ( - SELECT generate_series( - date_trunc('day', $1::timestamptz), - date_trunc('day', $2::timestamptz), - '1 day'::interval - )::timestamptz AS date + SELECT $1::timestamptz AS date + + UNION + + SELECT DISTINCT changed_at AS date + FROM user_status_changes + WHERE changed_at > $1::timestamptz + AND changed_at < $2::timestamptz + + UNION + + SELECT DISTINCT deleted_at AS date + FROM user_deleted + WHERE deleted_at > $1::timestamptz + AND deleted_at < $2::timestamptz + + UNION + + SELECT $2::timestamptz AS date ), latest_status_before_range AS ( - -- Get the last status change for each user before our date range - SELECT DISTINCT ON (user_id) - user_id, - new_status, - changed_at - FROM user_status_changes - WHERE changed_at < (SELECT MIN(date) FROM dates) - ORDER BY user_id, changed_at DESC + SELECT + DISTINCT usc.user_id, + usc.new_status, + usc.changed_at + FROM user_status_changes usc + LEFT JOIN user_deleted ud ON usc.user_id = ud.user_id + WHERE usc.changed_at < $1::timestamptz + AND (ud.user_id IS NULL OR ud.deleted_at > $1::timestamptz) + ORDER BY usc.user_id, usc.changed_at DESC +), +status_changes_during_range AS ( + SELECT + usc.user_id, + usc.new_status, + usc.changed_at + FROM user_status_changes usc + LEFT JOIN user_deleted ud ON usc.user_id = ud.user_id + WHERE usc.changed_at >= $1::timestamptz + AND usc.changed_at <= $2::timestamptz + AND (ud.user_id IS NULL OR usc.changed_at < ud.deleted_at) ), all_status_changes AS ( - -- Combine status changes before and during our range SELECT user_id, new_status, @@ -3126,42 +3151,36 @@ all_status_changes AS ( user_id, new_status, changed_at - FROM user_status_changes - WHERE changed_at < $2::timestamptz + FROM status_changes_during_range ), -daily_counts AS ( - SELECT - d.date, - asc1.new_status, - -- For each date and status, count users whose most recent status change - -- (up to that date) matches this status AND who weren't deleted by that date - COUNT(*) FILTER ( - WHERE asc1.changed_at = ( - SELECT MAX(changed_at) - FROM all_status_changes asc2 - WHERE asc2.user_id = asc1.user_id - AND asc2.changed_at <= d.date - ) - AND NOT EXISTS ( - SELECT 1 - FROM user_deleted ud - WHERE ud.user_id = asc1.user_id - AND ud.deleted_at <= d.date - ) - )::bigint AS count - FROM dates d - CROSS JOIN all_status_changes asc1 - GROUP BY d.date, asc1.new_status +ranked_status_change_per_user_per_date AS ( + SELECT + d.date, + asc1.user_id, + ROW_NUMBER() OVER (PARTITION BY d.date, asc1.user_id ORDER BY asc1.changed_at DESC) AS rn, + asc1.new_status + FROM dates d + LEFT JOIN all_status_changes asc1 ON asc1.changed_at <= d.date ) SELECT - date::timestamptz AS date, - new_status AS status, - count -FROM daily_counts -WHERE count > 0 -ORDER BY - status ASC, - date ASC + date, + statuses.new_status AS status, + COUNT(rscpupd.user_id) FILTER ( + WHERE rscpupd.rn = 1 + AND ( + rscpupd.new_status = statuses.new_status + AND ( + -- Include users who haven't been deleted + NOT EXISTS (SELECT 1 FROM user_deleted WHERE user_id = rscpupd.user_id) + OR + -- Or users whose deletion date is after the current date we're looking at + rscpupd.date < (SELECT deleted_at FROM user_deleted WHERE user_id = rscpupd.user_id) + ) + ) + ) AS count +FROM ranked_status_change_per_user_per_date rscpupd +CROSS JOIN (select distinct new_status FROM all_status_changes) statuses +GROUP BY date, statuses.new_status ` type GetUserStatusChangesParams struct { diff --git a/coderd/database/queries/insights.sql b/coderd/database/queries/insights.sql index 04e74bf7f166e..c721ebcc79227 100644 --- a/coderd/database/queries/insights.sql +++ b/coderd/database/queries/insights.sql @@ -774,24 +774,49 @@ GROUP BY utp.num, utp.template_ids, utp.name, utp.type, utp.display_name, utp.de -- name: GetUserStatusChanges :many WITH dates AS ( - SELECT generate_series( - date_trunc('day', @start_time::timestamptz), - date_trunc('day', @end_time::timestamptz), - '1 day'::interval - )::timestamptz AS date + SELECT @start_time::timestamptz AS date + + UNION + + SELECT DISTINCT changed_at AS date + FROM user_status_changes + WHERE changed_at > @start_time::timestamptz + AND changed_at < @end_time::timestamptz + + UNION + + SELECT DISTINCT deleted_at AS date + FROM user_deleted + WHERE deleted_at > @start_time::timestamptz + AND deleted_at < @end_time::timestamptz + + UNION + + SELECT @end_time::timestamptz AS date ), latest_status_before_range AS ( - -- Get the last status change for each user before our date range - SELECT DISTINCT ON (user_id) - user_id, - new_status, - changed_at - FROM user_status_changes - WHERE changed_at < (SELECT MIN(date) FROM dates) - ORDER BY user_id, changed_at DESC + SELECT + DISTINCT usc.user_id, + usc.new_status, + usc.changed_at + FROM user_status_changes usc + LEFT JOIN user_deleted ud ON usc.user_id = ud.user_id + WHERE usc.changed_at < @start_time::timestamptz + AND (ud.user_id IS NULL OR ud.deleted_at > @start_time::timestamptz) + ORDER BY usc.user_id, usc.changed_at DESC +), +status_changes_during_range AS ( + SELECT + usc.user_id, + usc.new_status, + usc.changed_at + FROM user_status_changes usc + LEFT JOIN user_deleted ud ON usc.user_id = ud.user_id + WHERE usc.changed_at >= @start_time::timestamptz + AND usc.changed_at <= @end_time::timestamptz + AND (ud.user_id IS NULL OR usc.changed_at < ud.deleted_at) ), all_status_changes AS ( - -- Combine status changes before and during our range SELECT user_id, new_status, @@ -804,39 +829,34 @@ all_status_changes AS ( user_id, new_status, changed_at - FROM user_status_changes - WHERE changed_at < @end_time::timestamptz + FROM status_changes_during_range ), -daily_counts AS ( - SELECT - d.date, - asc1.new_status, - -- For each date and status, count users whose most recent status change - -- (up to that date) matches this status AND who weren't deleted by that date - COUNT(*) FILTER ( - WHERE asc1.changed_at = ( - SELECT MAX(changed_at) - FROM all_status_changes asc2 - WHERE asc2.user_id = asc1.user_id - AND asc2.changed_at <= d.date - ) - AND NOT EXISTS ( - SELECT 1 - FROM user_deleted ud - WHERE ud.user_id = asc1.user_id - AND ud.deleted_at <= d.date - ) - )::bigint AS count - FROM dates d - CROSS JOIN all_status_changes asc1 - GROUP BY d.date, asc1.new_status +ranked_status_change_per_user_per_date AS ( + SELECT + d.date, + asc1.user_id, + ROW_NUMBER() OVER (PARTITION BY d.date, asc1.user_id ORDER BY asc1.changed_at DESC) AS rn, + asc1.new_status + FROM dates d + LEFT JOIN all_status_changes asc1 ON asc1.changed_at <= d.date ) SELECT - date::timestamptz AS date, - new_status AS status, - count -FROM daily_counts -WHERE count > 0 -ORDER BY - status ASC, - date ASC; + date, + statuses.new_status AS status, + COUNT(rscpupd.user_id) FILTER ( + WHERE rscpupd.rn = 1 + AND ( + rscpupd.new_status = statuses.new_status + AND ( + -- Include users who haven't been deleted + NOT EXISTS (SELECT 1 FROM user_deleted WHERE user_id = rscpupd.user_id) + OR + -- Or users whose deletion date is after the current date we're looking at + rscpupd.date < (SELECT deleted_at FROM user_deleted WHERE user_id = rscpupd.user_id) + ) + ) + ) AS count +FROM ranked_status_change_per_user_per_date rscpupd +CROSS JOIN (select distinct new_status FROM all_status_changes) statuses +GROUP BY date, statuses.new_status; + From 254d43676788a2db6f0ad9a66199d2ee8b677b63 Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Thu, 2 Jan 2025 17:31:57 +0000 Subject: [PATCH 17/32] renumber migrations --- ...tatus_changes.down.sql => 000281_user_status_changes.down.sql} | 0 ...er_status_changes.up.sql => 000281_user_status_changes.up.sql} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename coderd/database/migrations/{000280_user_status_changes.down.sql => 000281_user_status_changes.down.sql} (100%) rename coderd/database/migrations/{000280_user_status_changes.up.sql => 000281_user_status_changes.up.sql} (100%) diff --git a/coderd/database/migrations/000280_user_status_changes.down.sql b/coderd/database/migrations/000281_user_status_changes.down.sql similarity index 100% rename from coderd/database/migrations/000280_user_status_changes.down.sql rename to coderd/database/migrations/000281_user_status_changes.down.sql diff --git a/coderd/database/migrations/000280_user_status_changes.up.sql b/coderd/database/migrations/000281_user_status_changes.up.sql similarity index 100% rename from coderd/database/migrations/000280_user_status_changes.up.sql rename to coderd/database/migrations/000281_user_status_changes.up.sql From 3e86522c4ba719114fa05a9c85578d50d46ef78e Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Fri, 3 Jan 2025 06:11:55 +0000 Subject: [PATCH 18/32] add partial fixture for CI --- .../000281_user_status_changes.up.sql | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 coderd/database/migrations/testdata/fixtures/000281_user_status_changes.up.sql diff --git a/coderd/database/migrations/testdata/fixtures/000281_user_status_changes.up.sql b/coderd/database/migrations/testdata/fixtures/000281_user_status_changes.up.sql new file mode 100644 index 0000000000000..3f7bcc21b16f0 --- /dev/null +++ b/coderd/database/migrations/testdata/fixtures/000281_user_status_changes.up.sql @@ -0,0 +1,44 @@ +INSERT INTO + users ( + id, + email, + username, + hashed_password, + created_at, + updated_at, + status, + rbac_roles, + login_type, + avatar_url, + deleted, + last_seen_at, + quiet_hours_schedule, + theme_preference, + name, + github_com_user_id, + hashed_one_time_passcode, + one_time_passcode_expires_at + ) + VALUES ( + '5755e622-fadd-44ca-98da-5df070491844', -- uuid + 'test@example.com', + 'testuser', + 'hashed_password', + '2024-01-01 00:00:00', + '2024-01-01 00:00:00', + 'active', + '{}', + 'password', + '', + false, + '2024-01-01 00:00:00', + '', + '', + '', + 123, + NULL, + NULL + ); + +UPDATE users SET status = 'dormant', updated_at = '2024-01-01 01:00:00' WHERE id = '5755e622-fadd-44ca-98da-5df070491844'; +UPDATE users SET deleted = true, updated_at = '2024-01-01 02:00:00' WHERE id = '5755e622-fadd-44ca-98da-5df070491844'; From 2e49e4c6443ef26bc93c97c71ca72e9920296c7d Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Fri, 3 Jan 2025 06:22:27 +0000 Subject: [PATCH 19/32] fix migration numbers --- ...tatus_changes.down.sql => 000282_user_status_changes.down.sql} | 0 ...er_status_changes.up.sql => 000282_user_status_changes.up.sql} | 0 ...er_status_changes.up.sql => 000282_user_status_changes.up.sql} | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename coderd/database/migrations/{000281_user_status_changes.down.sql => 000282_user_status_changes.down.sql} (100%) rename coderd/database/migrations/{000281_user_status_changes.up.sql => 000282_user_status_changes.up.sql} (100%) rename coderd/database/migrations/testdata/fixtures/{000281_user_status_changes.up.sql => 000282_user_status_changes.up.sql} (100%) diff --git a/coderd/database/migrations/000281_user_status_changes.down.sql b/coderd/database/migrations/000282_user_status_changes.down.sql similarity index 100% rename from coderd/database/migrations/000281_user_status_changes.down.sql rename to coderd/database/migrations/000282_user_status_changes.down.sql diff --git a/coderd/database/migrations/000281_user_status_changes.up.sql b/coderd/database/migrations/000282_user_status_changes.up.sql similarity index 100% rename from coderd/database/migrations/000281_user_status_changes.up.sql rename to coderd/database/migrations/000282_user_status_changes.up.sql diff --git a/coderd/database/migrations/testdata/fixtures/000281_user_status_changes.up.sql b/coderd/database/migrations/testdata/fixtures/000282_user_status_changes.up.sql similarity index 100% rename from coderd/database/migrations/testdata/fixtures/000281_user_status_changes.up.sql rename to coderd/database/migrations/testdata/fixtures/000282_user_status_changes.up.sql From ff59729d34c60e80942985463b1a93b042db2f0a Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Fri, 3 Jan 2025 06:51:17 +0000 Subject: [PATCH 20/32] rename and document sql function --- coderd/apidoc/docs.go | 4 +- coderd/apidoc/swagger.json | 4 +- coderd/database/dbauthz/dbauthz.go | 4 +- coderd/database/dbauthz/dbauthz_test.go | 4 +- coderd/database/dbmem/dbmem.go | 8 +-- coderd/database/dbmetrics/querymetrics.go | 6 +- coderd/database/dbmock/dbmock.go | 14 ++--- coderd/database/querier.go | 30 ++++++++- coderd/database/querier_test.go | 16 ++--- coderd/database/queries.sql.go | 62 ++++++++++++++----- coderd/database/queries/insights.sql | 50 ++++++++++++--- coderd/insights.go | 6 +- codersdk/insights.go | 12 ++-- docs/reference/api/insights.md | 6 +- docs/reference/api/schemas.md | 2 +- site/src/api/api.ts | 2 +- site/src/api/typesGenerated.ts | 4 +- .../GeneralSettingsPageView.tsx | 4 +- 18 files changed, 165 insertions(+), 73 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 983aa7227a4a5..0243e8310c066 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -1426,7 +1426,7 @@ const docTemplate = `{ "200": { "description": "OK", "schema": { - "$ref": "#/definitions/codersdk.GetUserStatusChangesResponse" + "$ref": "#/definitions/codersdk.GetUserStatusCountsOverTimeResponse" } } } @@ -11241,7 +11241,7 @@ const docTemplate = `{ } } }, - "codersdk.GetUserStatusChangesResponse": { + "codersdk.GetUserStatusCountsOverTimeResponse": { "type": "object", "properties": { "status_counts": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index d700a3089114a..5d9dda0664489 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -1243,7 +1243,7 @@ "200": { "description": "OK", "schema": { - "$ref": "#/definitions/codersdk.GetUserStatusChangesResponse" + "$ref": "#/definitions/codersdk.GetUserStatusCountsOverTimeResponse" } } } @@ -10084,7 +10084,7 @@ } } }, - "codersdk.GetUserStatusChangesResponse": { + "codersdk.GetUserStatusCountsOverTimeResponse": { "type": "object", "properties": { "status_counts": { diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 7dff12529fe8a..be04c6170c986 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -2421,11 +2421,11 @@ func (q *querier) GetUserNotificationPreferences(ctx context.Context, userID uui return q.db.GetUserNotificationPreferences(ctx, userID) } -func (q *querier) GetUserStatusChanges(ctx context.Context, arg database.GetUserStatusChangesParams) ([]database.GetUserStatusChangesRow, error) { +func (q *querier) GetUserStatusCountsOverTime(ctx context.Context, arg database.GetUserStatusCountsOverTimeParams) ([]database.GetUserStatusCountsOverTimeRow, error) { if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceUser); err != nil { return nil, err } - return q.db.GetUserStatusChanges(ctx, arg) + return q.db.GetUserStatusCountsOverTime(ctx, arg) } func (q *querier) GetUserWorkspaceBuildParameters(ctx context.Context, params database.GetUserWorkspaceBuildParametersParams) ([]database.GetUserWorkspaceBuildParametersRow, error) { diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index b1c5898a82e34..3bc1f80519f0a 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -1708,8 +1708,8 @@ func (s *MethodTestSuite) TestUser() { rbac.ResourceTemplate.InOrg(orgID), policy.ActionRead, ) })) - s.Run("GetUserStatusChanges", s.Subtest(func(db database.Store, check *expects) { - check.Args(database.GetUserStatusChangesParams{ + s.Run("GetUserStatusCountsOverTime", s.Subtest(func(db database.Store, check *expects) { + check.Args(database.GetUserStatusCountsOverTimeParams{ StartTime: time.Now().Add(-time.Hour * 24 * 30), EndTime: time.Now(), }).Asserts(rbac.ResourceUser, policy.ActionRead) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 07f77ccb460cb..736823553af4a 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -5671,7 +5671,7 @@ func (q *FakeQuerier) GetUserNotificationPreferences(_ context.Context, userID u return out, nil } -func (q *FakeQuerier) GetUserStatusChanges(_ context.Context, arg database.GetUserStatusChangesParams) ([]database.GetUserStatusChangesRow, error) { +func (q *FakeQuerier) GetUserStatusCountsOverTime(_ context.Context, arg database.GetUserStatusCountsOverTimeParams) ([]database.GetUserStatusCountsOverTimeRow, error) { q.mutex.RLock() defer q.mutex.RUnlock() @@ -5680,16 +5680,16 @@ func (q *FakeQuerier) GetUserStatusChanges(_ context.Context, arg database.GetUs return nil, err } - result := make([]database.GetUserStatusChangesRow, 0) + result := make([]database.GetUserStatusCountsOverTimeRow, 0) for _, change := range q.userStatusChanges { if change.ChangedAt.Before(arg.StartTime) || change.ChangedAt.After(arg.EndTime) { continue } date := time.Date(change.ChangedAt.Year(), change.ChangedAt.Month(), change.ChangedAt.Day(), 0, 0, 0, 0, time.UTC) - if !slices.ContainsFunc(result, func(r database.GetUserStatusChangesRow) bool { + if !slices.ContainsFunc(result, func(r database.GetUserStatusCountsOverTimeRow) bool { return r.Status == change.NewStatus && r.Date.Equal(date) }) { - result = append(result, database.GetUserStatusChangesRow{ + result = append(result, database.GetUserStatusCountsOverTimeRow{ Status: change.NewStatus, Date: date, Count: 1, diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index f3866cbd6a482..bf5b8954cfeb0 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -1344,10 +1344,10 @@ func (m queryMetricsStore) GetUserNotificationPreferences(ctx context.Context, u return r0, r1 } -func (m queryMetricsStore) GetUserStatusChanges(ctx context.Context, arg database.GetUserStatusChangesParams) ([]database.GetUserStatusChangesRow, error) { +func (m queryMetricsStore) GetUserStatusCountsOverTime(ctx context.Context, arg database.GetUserStatusCountsOverTimeParams) ([]database.GetUserStatusCountsOverTimeRow, error) { start := time.Now() - r0, r1 := m.s.GetUserStatusChanges(ctx, arg) - m.queryLatencies.WithLabelValues("GetUserStatusChanges").Observe(time.Since(start).Seconds()) + r0, r1 := m.s.GetUserStatusCountsOverTime(ctx, arg) + m.queryLatencies.WithLabelValues("GetUserStatusCountsOverTime").Observe(time.Since(start).Seconds()) return r0, r1 } diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index 6abc14205a22f..4f743a453dc0d 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -2825,19 +2825,19 @@ func (mr *MockStoreMockRecorder) GetUserNotificationPreferences(arg0, arg1 any) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserNotificationPreferences", reflect.TypeOf((*MockStore)(nil).GetUserNotificationPreferences), arg0, arg1) } -// GetUserStatusChanges mocks base method. -func (m *MockStore) GetUserStatusChanges(arg0 context.Context, arg1 database.GetUserStatusChangesParams) ([]database.GetUserStatusChangesRow, error) { +// GetUserStatusCountsOverTime mocks base method. +func (m *MockStore) GetUserStatusCountsOverTime(arg0 context.Context, arg1 database.GetUserStatusCountsOverTimeParams) ([]database.GetUserStatusCountsOverTimeRow, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetUserStatusChanges", arg0, arg1) - ret0, _ := ret[0].([]database.GetUserStatusChangesRow) + ret := m.ctrl.Call(m, "GetUserStatusCountsOverTime", arg0, arg1) + ret0, _ := ret[0].([]database.GetUserStatusCountsOverTimeRow) ret1, _ := ret[1].(error) return ret0, ret1 } -// GetUserStatusChanges indicates an expected call of GetUserStatusChanges. -func (mr *MockStoreMockRecorder) GetUserStatusChanges(arg0, arg1 any) *gomock.Call { +// GetUserStatusCountsOverTime indicates an expected call of GetUserStatusCountsOverTime. +func (mr *MockStoreMockRecorder) GetUserStatusCountsOverTime(arg0, arg1 any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserStatusChanges", reflect.TypeOf((*MockStore)(nil).GetUserStatusChanges), arg0, arg1) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserStatusCountsOverTime", reflect.TypeOf((*MockStore)(nil).GetUserStatusCountsOverTime), arg0, arg1) } // GetUserWorkspaceBuildParameters mocks base method. diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 0e3459882c73b..f0a24627ecc33 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -289,7 +289,35 @@ type sqlcQuerier interface { GetUserLinkByUserIDLoginType(ctx context.Context, arg GetUserLinkByUserIDLoginTypeParams) (UserLink, error) GetUserLinksByUserID(ctx context.Context, userID uuid.UUID) ([]UserLink, error) GetUserNotificationPreferences(ctx context.Context, userID uuid.UUID) ([]NotificationPreference, error) - GetUserStatusChanges(ctx context.Context, arg GetUserStatusChangesParams) ([]GetUserStatusChangesRow, error) + // GetUserStatusCountsOverTime returns the count of users in each status over time. + // The time range is inclusively defined by the start_time and end_time parameters. + // + // Bucketing: + // Between the start_time and end_time, we include each timestamp where a user's status changed or they were deleted. + // We do not bucket these results by day or some other time unit. This is because such bucketing would hide potentially + // important patterns. If a user was active for 23 hours and 59 minutes, and then suspended, a daily bucket would hide this. + // A daily bucket would also have required us to carefully manage the timezone of the bucket based on the timezone of the user. + // + // Accumulation: + // We do not start counting from 0 at the start_time. We check the last status change before the start_time for each user. As such, + // the result shows the total number of users in each status on any particular day. + // dates_of_interest defines all points in time that are relevant to the query. + // It includes the start_time, all status changes, all deletions, and the end_time. + // latest_status_before_range defines the status of each user before the start_time. + // We do not include users who were deleted before the start_time. We use this to ensure that + // we correctly count users prior to the start_time for a complete graph. + // status_changes_during_range defines the status of each user during the start_time and end_time. + // If a user is deleted during the time range, we count status changes prior to the deletion. + // Theoretically, it should probably not be possible to update the status of a deleted user, but we + // need to ensure that this is enforced, so that a change in business logic later does not break this graph. + // relevant_status_changes defines the status of each user at any point in time. + // It includes the status of each user before the start_time, and the status of each user during the start_time and end_time. + // statuses defines all the distinct statuses that were present just before and during the time range. + // This is used to ensure that we have a series for every relevant status. + // We only want to count the latest status change for each user on each date and then filter them by the relevant status. + // We use the row_number function to ensure that we only count the latest status change for each user on each date. + // We then filter the status changes by the relevant status in the final select statement below. + GetUserStatusCountsOverTime(ctx context.Context, arg GetUserStatusCountsOverTimeParams) ([]GetUserStatusCountsOverTimeRow, error) GetUserWorkspaceBuildParameters(ctx context.Context, arg GetUserWorkspaceBuildParametersParams) ([]GetUserWorkspaceBuildParametersRow, error) // This will never return deleted users. GetUsers(ctx context.Context, arg GetUsersParams) ([]GetUsersRow, error) diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index 6ace33f74a96c..8d0d7edf312f6 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -2256,7 +2256,7 @@ func TestGroupRemovalTrigger(t *testing.T) { }, db2sdk.List(extraUserGroups, onlyGroupIDs)) } -func TestGetUserStatusChanges(t *testing.T) { +func TestGetUserStatusCountsOverTime(t *testing.T) { t.Parallel() if !dbtestutil.WillUsePostgres() { @@ -2294,7 +2294,7 @@ func TestGetUserStatusChanges(t *testing.T) { end := dbtime.Now() start := end.Add(-30 * 24 * time.Hour) - counts, err := db.GetUserStatusChanges(ctx, database.GetUserStatusChangesParams{ + counts, err := db.GetUserStatusCountsOverTime(ctx, database.GetUserStatusCountsOverTimeParams{ StartTime: start, EndTime: end, }) @@ -2342,7 +2342,7 @@ func TestGetUserStatusChanges(t *testing.T) { }) // Query for the last 30 days - userStatusChanges, err := db.GetUserStatusChanges(ctx, database.GetUserStatusChangesParams{ + userStatusChanges, err := db.GetUserStatusCountsOverTime(ctx, database.GetUserStatusCountsOverTimeParams{ StartTime: createdAt, EndTime: today, }) @@ -2510,7 +2510,7 @@ func TestGetUserStatusChanges(t *testing.T) { require.NoError(t, err) // Query for the last 5 days - userStatusChanges, err := db.GetUserStatusChanges(ctx, database.GetUserStatusChangesParams{ + userStatusChanges, err := db.GetUserStatusCountsOverTime(ctx, database.GetUserStatusCountsOverTimeParams{ StartTime: createdAt, EndTime: today, }) @@ -2639,7 +2639,7 @@ func TestGetUserStatusChanges(t *testing.T) { }) require.NoError(t, err) - userStatusChanges, err := db.GetUserStatusChanges(ctx, database.GetUserStatusChangesParams{ + userStatusChanges, err := db.GetUserStatusCountsOverTime(ctx, database.GetUserStatusCountsOverTimeParams{ StartTime: createdAt, EndTime: today, }) @@ -2715,7 +2715,7 @@ func TestGetUserStatusChanges(t *testing.T) { UpdatedAt: createdAt, }) - userStatusChanges, err := db.GetUserStatusChanges(ctx, database.GetUserStatusChangesParams{ + userStatusChanges, err := db.GetUserStatusCountsOverTime(ctx, database.GetUserStatusCountsOverTimeParams{ StartTime: createdAt.Add(time.Hour * 24), EndTime: today, }) @@ -2742,7 +2742,7 @@ func TestGetUserStatusChanges(t *testing.T) { err = db.UpdateUserDeletedByID(ctx, user.ID) require.NoError(t, err) - userStatusChanges, err := db.GetUserStatusChanges(ctx, database.GetUserStatusChangesParams{ + userStatusChanges, err := db.GetUserStatusCountsOverTime(ctx, database.GetUserStatusCountsOverTimeParams{ StartTime: today.Add(time.Hour * 24), EndTime: today.Add(time.Hour * 48), }) @@ -2764,7 +2764,7 @@ func TestGetUserStatusChanges(t *testing.T) { err := db.UpdateUserDeletedByID(ctx, user.ID) require.NoError(t, err) - userStatusChanges, err := db.GetUserStatusChanges(ctx, database.GetUserStatusChangesParams{ + userStatusChanges, err := db.GetUserStatusCountsOverTime(ctx, database.GetUserStatusCountsOverTimeParams{ StartTime: createdAt, EndTime: today.Add(time.Hour * 24), }) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 8d271b8f77293..02f848a45f19a 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3094,8 +3094,9 @@ func (q *sqlQuerier) GetUserLatencyInsights(ctx context.Context, arg GetUserLate return items, nil } -const getUserStatusChanges = `-- name: GetUserStatusChanges :many -WITH dates AS ( +const getUserStatusCountsOverTime = `-- name: GetUserStatusCountsOverTime :many +WITH +dates_of_interest AS ( SELECT $1::timestamptz AS date UNION @@ -3138,7 +3139,7 @@ status_changes_during_range AS ( AND usc.changed_at <= $2::timestamptz AND (ud.user_id IS NULL OR usc.changed_at < ud.deleted_at) ), -all_status_changes AS ( +relevant_status_changes AS ( SELECT user_id, new_status, @@ -3153,14 +3154,17 @@ all_status_changes AS ( changed_at FROM status_changes_during_range ), +statuses AS ( + SELECT DISTINCT new_status FROM relevant_status_changes +), ranked_status_change_per_user_per_date AS ( SELECT d.date, - asc1.user_id, - ROW_NUMBER() OVER (PARTITION BY d.date, asc1.user_id ORDER BY asc1.changed_at DESC) AS rn, - asc1.new_status - FROM dates d - LEFT JOIN all_status_changes asc1 ON asc1.changed_at <= d.date + rsc1.user_id, + ROW_NUMBER() OVER (PARTITION BY d.date, rsc1.user_id ORDER BY rsc1.changed_at DESC) AS rn, + rsc1.new_status + FROM dates_of_interest d + LEFT JOIN relevant_status_changes rsc1 ON rsc1.changed_at <= d.date ) SELECT date, @@ -3179,30 +3183,58 @@ SELECT ) ) AS count FROM ranked_status_change_per_user_per_date rscpupd -CROSS JOIN (select distinct new_status FROM all_status_changes) statuses +CROSS JOIN statuses GROUP BY date, statuses.new_status ` -type GetUserStatusChangesParams struct { +type GetUserStatusCountsOverTimeParams struct { StartTime time.Time `db:"start_time" json:"start_time"` EndTime time.Time `db:"end_time" json:"end_time"` } -type GetUserStatusChangesRow struct { +type GetUserStatusCountsOverTimeRow struct { Date time.Time `db:"date" json:"date"` Status UserStatus `db:"status" json:"status"` Count int64 `db:"count" json:"count"` } -func (q *sqlQuerier) GetUserStatusChanges(ctx context.Context, arg GetUserStatusChangesParams) ([]GetUserStatusChangesRow, error) { - rows, err := q.db.QueryContext(ctx, getUserStatusChanges, arg.StartTime, arg.EndTime) +// GetUserStatusCountsOverTime returns the count of users in each status over time. +// The time range is inclusively defined by the start_time and end_time parameters. +// +// Bucketing: +// Between the start_time and end_time, we include each timestamp where a user's status changed or they were deleted. +// We do not bucket these results by day or some other time unit. This is because such bucketing would hide potentially +// important patterns. If a user was active for 23 hours and 59 minutes, and then suspended, a daily bucket would hide this. +// A daily bucket would also have required us to carefully manage the timezone of the bucket based on the timezone of the user. +// +// Accumulation: +// We do not start counting from 0 at the start_time. We check the last status change before the start_time for each user. As such, +// the result shows the total number of users in each status on any particular day. +// dates_of_interest defines all points in time that are relevant to the query. +// It includes the start_time, all status changes, all deletions, and the end_time. +// latest_status_before_range defines the status of each user before the start_time. +// We do not include users who were deleted before the start_time. We use this to ensure that +// we correctly count users prior to the start_time for a complete graph. +// status_changes_during_range defines the status of each user during the start_time and end_time. +// If a user is deleted during the time range, we count status changes prior to the deletion. +// Theoretically, it should probably not be possible to update the status of a deleted user, but we +// need to ensure that this is enforced, so that a change in business logic later does not break this graph. +// relevant_status_changes defines the status of each user at any point in time. +// It includes the status of each user before the start_time, and the status of each user during the start_time and end_time. +// statuses defines all the distinct statuses that were present just before and during the time range. +// This is used to ensure that we have a series for every relevant status. +// We only want to count the latest status change for each user on each date and then filter them by the relevant status. +// We use the row_number function to ensure that we only count the latest status change for each user on each date. +// We then filter the status changes by the relevant status in the final select statement below. +func (q *sqlQuerier) GetUserStatusCountsOverTime(ctx context.Context, arg GetUserStatusCountsOverTimeParams) ([]GetUserStatusCountsOverTimeRow, error) { + rows, err := q.db.QueryContext(ctx, getUserStatusCountsOverTime, arg.StartTime, arg.EndTime) if err != nil { return nil, err } defer rows.Close() - var items []GetUserStatusChangesRow + var items []GetUserStatusCountsOverTimeRow for rows.Next() { - var i GetUserStatusChangesRow + var i GetUserStatusCountsOverTimeRow if err := rows.Scan(&i.Date, &i.Status, &i.Count); err != nil { return nil, err } diff --git a/coderd/database/queries/insights.sql b/coderd/database/queries/insights.sql index c721ebcc79227..d599e2989a56e 100644 --- a/coderd/database/queries/insights.sql +++ b/coderd/database/queries/insights.sql @@ -772,8 +772,23 @@ FROM unique_template_params utp JOIN workspace_build_parameters wbp ON (utp.workspace_build_ids @> ARRAY[wbp.workspace_build_id] AND utp.name = wbp.name) GROUP BY utp.num, utp.template_ids, utp.name, utp.type, utp.display_name, utp.description, utp.options, wbp.value; --- name: GetUserStatusChanges :many -WITH dates AS ( +-- name: GetUserStatusCountsOverTime :many +-- GetUserStatusCountsOverTime returns the count of users in each status over time. +-- The time range is inclusively defined by the start_time and end_time parameters. +-- +-- Bucketing: +-- Between the start_time and end_time, we include each timestamp where a user's status changed or they were deleted. +-- We do not bucket these results by day or some other time unit. This is because such bucketing would hide potentially +-- important patterns. If a user was active for 23 hours and 59 minutes, and then suspended, a daily bucket would hide this. +-- A daily bucket would also have required us to carefully manage the timezone of the bucket based on the timezone of the user. +-- +-- Accumulation: +-- We do not start counting from 0 at the start_time. We check the last status change before the start_time for each user. As such, +-- the result shows the total number of users in each status on any particular day. +WITH +-- dates_of_interest defines all points in time that are relevant to the query. +-- It includes the start_time, all status changes, all deletions, and the end_time. +dates_of_interest AS ( SELECT @start_time::timestamptz AS date UNION @@ -794,6 +809,9 @@ WITH dates AS ( SELECT @end_time::timestamptz AS date ), +-- latest_status_before_range defines the status of each user before the start_time. +-- We do not include users who were deleted before the start_time. We use this to ensure that +-- we correctly count users prior to the start_time for a complete graph. latest_status_before_range AS ( SELECT DISTINCT usc.user_id, @@ -805,6 +823,10 @@ latest_status_before_range AS ( AND (ud.user_id IS NULL OR ud.deleted_at > @start_time::timestamptz) ORDER BY usc.user_id, usc.changed_at DESC ), +-- status_changes_during_range defines the status of each user during the start_time and end_time. +-- If a user is deleted during the time range, we count status changes prior to the deletion. +-- Theoretically, it should probably not be possible to update the status of a deleted user, but we +-- need to ensure that this is enforced, so that a change in business logic later does not break this graph. status_changes_during_range AS ( SELECT usc.user_id, @@ -816,7 +838,9 @@ status_changes_during_range AS ( AND usc.changed_at <= @end_time::timestamptz AND (ud.user_id IS NULL OR usc.changed_at < ud.deleted_at) ), -all_status_changes AS ( +-- relevant_status_changes defines the status of each user at any point in time. +-- It includes the status of each user before the start_time, and the status of each user during the start_time and end_time. +relevant_status_changes AS ( SELECT user_id, new_status, @@ -831,14 +855,22 @@ all_status_changes AS ( changed_at FROM status_changes_during_range ), +-- statuses defines all the distinct statuses that were present just before and during the time range. +-- This is used to ensure that we have a series for every relevant status. +statuses AS ( + SELECT DISTINCT new_status FROM relevant_status_changes +), +-- We only want to count the latest status change for each user on each date and then filter them by the relevant status. +-- We use the row_number function to ensure that we only count the latest status change for each user on each date. +-- We then filter the status changes by the relevant status in the final select statement below. ranked_status_change_per_user_per_date AS ( SELECT d.date, - asc1.user_id, - ROW_NUMBER() OVER (PARTITION BY d.date, asc1.user_id ORDER BY asc1.changed_at DESC) AS rn, - asc1.new_status - FROM dates d - LEFT JOIN all_status_changes asc1 ON asc1.changed_at <= d.date + rsc1.user_id, + ROW_NUMBER() OVER (PARTITION BY d.date, rsc1.user_id ORDER BY rsc1.changed_at DESC) AS rn, + rsc1.new_status + FROM dates_of_interest d + LEFT JOIN relevant_status_changes rsc1 ON rsc1.changed_at <= d.date ) SELECT date, @@ -857,6 +889,6 @@ SELECT ) ) AS count FROM ranked_status_change_per_user_per_date rscpupd -CROSS JOIN (select distinct new_status FROM all_status_changes) statuses +CROSS JOIN statuses GROUP BY date, statuses.new_status; diff --git a/coderd/insights.go b/coderd/insights.go index 3cd9a364a2efe..10205439e2b4a 100644 --- a/coderd/insights.go +++ b/coderd/insights.go @@ -298,7 +298,7 @@ func (api *API) insightsUserLatency(rw http.ResponseWriter, r *http.Request) { // @Produce json // @Tags Insights // @Param tz_offset query int true "Time-zone offset (e.g. -2)" -// @Success 200 {object} codersdk.GetUserStatusChangesResponse +// @Success 200 {object} codersdk.GetUserStatusCountsOverTimeResponse // @Router /insights/user-status-counts-over-time [get] func (api *API) insightsUserStatusCountsOverTime(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() @@ -324,7 +324,7 @@ func (api *API) insightsUserStatusCountsOverTime(rw http.ResponseWriter, r *http // Always return 60 days of data (2 months). sixtyDaysAgo := nextHourInLoc.In(loc).Truncate(24*time.Hour).AddDate(0, 0, -60) - rows, err := api.Database.GetUserStatusChanges(ctx, database.GetUserStatusChangesParams{ + rows, err := api.Database.GetUserStatusCountsOverTime(ctx, database.GetUserStatusCountsOverTimeParams{ StartTime: sixtyDaysAgo, EndTime: nextHourInLoc, }) @@ -336,7 +336,7 @@ func (api *API) insightsUserStatusCountsOverTime(rw http.ResponseWriter, r *http return } - resp := codersdk.GetUserStatusChangesResponse{ + resp := codersdk.GetUserStatusCountsOverTimeResponse{ StatusCounts: make(map[codersdk.UserStatus][]codersdk.UserStatusChangeCount), } diff --git a/codersdk/insights.go b/codersdk/insights.go index b3a3cc728378a..3e285b5de0a58 100644 --- a/codersdk/insights.go +++ b/codersdk/insights.go @@ -283,7 +283,7 @@ func (c *Client) TemplateInsights(ctx context.Context, req TemplateInsightsReque return result, json.NewDecoder(resp.Body).Decode(&result) } -type GetUserStatusChangesResponse struct { +type GetUserStatusCountsOverTimeResponse struct { StatusCounts map[UserStatus][]UserStatusChangeCount `json:"status_counts"` } @@ -292,24 +292,24 @@ type UserStatusChangeCount struct { Count int64 `json:"count" example:"10"` } -type GetUserStatusChangesRequest struct { +type GetUserStatusCountsOverTimeRequest struct { Offset time.Time `json:"offset" format:"date-time"` } -func (c *Client) GetUserStatusChanges(ctx context.Context, req GetUserStatusChangesRequest) (GetUserStatusChangesResponse, error) { +func (c *Client) GetUserStatusCountsOverTime(ctx context.Context, req GetUserStatusCountsOverTimeRequest) (GetUserStatusCountsOverTimeResponse, error) { qp := url.Values{} qp.Add("offset", req.Offset.Format(insightsTimeLayout)) reqURL := fmt.Sprintf("/api/v2/insights/user-status-counts-over-time?%s", qp.Encode()) resp, err := c.Request(ctx, http.MethodGet, reqURL, nil) if err != nil { - return GetUserStatusChangesResponse{}, xerrors.Errorf("make request: %w", err) + return GetUserStatusCountsOverTimeResponse{}, xerrors.Errorf("make request: %w", err) } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - return GetUserStatusChangesResponse{}, ReadBodyAsError(resp) + return GetUserStatusCountsOverTimeResponse{}, ReadBodyAsError(resp) } - var result GetUserStatusChangesResponse + var result GetUserStatusCountsOverTimeResponse return result, json.NewDecoder(resp.Body).Decode(&result) } diff --git a/docs/reference/api/insights.md b/docs/reference/api/insights.md index f4f00ec24c058..1d37811a6784d 100644 --- a/docs/reference/api/insights.md +++ b/docs/reference/api/insights.md @@ -305,8 +305,8 @@ curl -X GET http://coder-server:8080/api/v2/insights/user-status-counts-over-tim ### Responses -| Status | Meaning | Description | Schema | -| ------ | ------------------------------------------------------- | ----------- | ---------------------------------------------------------------------------------------- | -| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | [codersdk.GetUserStatusChangesResponse](schemas.md#codersdkgetuserstatuschangesresponse) | +| Status | Meaning | Description | Schema | +| ------ | ------------------------------------------------------- | ----------- | ------------------------------------------------------------------------------------------------------ | +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | [codersdk.GetUserStatusCountsOverTimeResponse](schemas.md#codersdkgetuserstatuscountsovertimeresponse) | To perform this operation, you must be authenticated. [Learn more](authentication.md). diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index a735163e2bc57..6c9a9f64bc7da 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -3000,7 +3000,7 @@ Git clone makes use of this by parsing the URL from: 'Username for "https://gith |-------|--------|----------|--------------|-------------| | `key` | string | false | | | -## codersdk.GetUserStatusChangesResponse +## codersdk.GetUserStatusCountsOverTimeResponse ```json { diff --git a/site/src/api/api.ts b/site/src/api/api.ts index 16c0506774295..74ca5ce685007 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -2099,7 +2099,7 @@ class ApiMethods { getInsightsUserStatusCountsOverTime = async ( offset = Math.trunc(new Date().getTimezoneOffset() / 60), - ): Promise => { + ): Promise => { const searchParams = new URLSearchParams({ offset: offset.toString(), }); diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 6faaf18dadadc..238209593ea6b 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -883,12 +883,12 @@ export interface GenerateAPIKeyResponse { } // From codersdk/insights.go -export interface GetUserStatusChangesRequest { +export interface GetUserStatusCountsOverTimeRequest { readonly offset: string; } // From codersdk/insights.go -export interface GetUserStatusChangesResponse { +export interface GetUserStatusCountsOverTimeResponse { readonly status_counts: Record; } diff --git a/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPageView.tsx b/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPageView.tsx index bf663fecaa945..3680e823516a6 100644 --- a/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPageView.tsx +++ b/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPageView.tsx @@ -2,7 +2,7 @@ import AlertTitle from "@mui/material/AlertTitle"; import type { DAUsResponse, Experiments, - GetUserStatusChangesResponse, + GetUserStatusCountsOverTimeResponse, SerpentOption, } from "api/typesGenerated"; import { @@ -25,7 +25,7 @@ export type GeneralSettingsPageViewProps = { deploymentOptions: SerpentOption[]; deploymentDAUs?: DAUsResponse; deploymentDAUsError: unknown; - userStatusCountsOverTime?: GetUserStatusChangesResponse; + userStatusCountsOverTime?: GetUserStatusCountsOverTimeResponse; activeUserLimit?: number; readonly invalidExperiments: Experiments | string[]; readonly safeExperiments: Experiments | string[]; From b1ad074e5e3a5617a1f8bafd2cab0d53eee69a3d Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Fri, 3 Jan 2025 07:05:25 +0000 Subject: [PATCH 21/32] make gen --- site/src/api/typesGenerated.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 238209593ea6b..d0ffc1aade125 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -892,6 +892,16 @@ export interface GetUserStatusCountsOverTimeResponse { readonly status_counts: Record; } +// From codersdk/insights.go +export interface GetUserStatusCountsOverTimeRequest { + readonly offset: string; +} + +// From codersdk/insights.go +export interface GetUserStatusCountsOverTimeResponse { + readonly status_counts: Record; +} + // From codersdk/users.go export interface GetUsersResponse { readonly users: readonly User[]; From de4081f901dd3d742950c0657ae805e884bb4426 Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Thu, 9 Jan 2025 10:14:46 +0000 Subject: [PATCH 22/32] Remove unwanted comments from the generated interface --- coderd/database/querier.go | 16 -------------- coderd/database/queries.sql.go | 32 ++++++++++++++-------------- coderd/database/queries/insights.sql | 32 ++++++++++++++-------------- 3 files changed, 32 insertions(+), 48 deletions(-) diff --git a/coderd/database/querier.go b/coderd/database/querier.go index f0a24627ecc33..948c5f914ab61 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -301,22 +301,6 @@ type sqlcQuerier interface { // Accumulation: // We do not start counting from 0 at the start_time. We check the last status change before the start_time for each user. As such, // the result shows the total number of users in each status on any particular day. - // dates_of_interest defines all points in time that are relevant to the query. - // It includes the start_time, all status changes, all deletions, and the end_time. - // latest_status_before_range defines the status of each user before the start_time. - // We do not include users who were deleted before the start_time. We use this to ensure that - // we correctly count users prior to the start_time for a complete graph. - // status_changes_during_range defines the status of each user during the start_time and end_time. - // If a user is deleted during the time range, we count status changes prior to the deletion. - // Theoretically, it should probably not be possible to update the status of a deleted user, but we - // need to ensure that this is enforced, so that a change in business logic later does not break this graph. - // relevant_status_changes defines the status of each user at any point in time. - // It includes the status of each user before the start_time, and the status of each user during the start_time and end_time. - // statuses defines all the distinct statuses that were present just before and during the time range. - // This is used to ensure that we have a series for every relevant status. - // We only want to count the latest status change for each user on each date and then filter them by the relevant status. - // We use the row_number function to ensure that we only count the latest status change for each user on each date. - // We then filter the status changes by the relevant status in the final select statement below. GetUserStatusCountsOverTime(ctx context.Context, arg GetUserStatusCountsOverTimeParams) ([]GetUserStatusCountsOverTimeRow, error) GetUserWorkspaceBuildParameters(ctx context.Context, arg GetUserWorkspaceBuildParametersParams) ([]GetUserWorkspaceBuildParametersRow, error) // This will never return deleted users. diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 02f848a45f19a..43adacb2ceb75 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3096,6 +3096,8 @@ func (q *sqlQuerier) GetUserLatencyInsights(ctx context.Context, arg GetUserLate const getUserStatusCountsOverTime = `-- name: GetUserStatusCountsOverTime :many WITH + -- dates_of_interest defines all points in time that are relevant to the query. + -- It includes the start_time, all status changes, all deletions, and the end_time. dates_of_interest AS ( SELECT $1::timestamptz AS date @@ -3117,6 +3119,9 @@ dates_of_interest AS ( SELECT $2::timestamptz AS date ), + -- latest_status_before_range defines the status of each user before the start_time. + -- We do not include users who were deleted before the start_time. We use this to ensure that + -- we correctly count users prior to the start_time for a complete graph. latest_status_before_range AS ( SELECT DISTINCT usc.user_id, @@ -3128,6 +3133,10 @@ latest_status_before_range AS ( AND (ud.user_id IS NULL OR ud.deleted_at > $1::timestamptz) ORDER BY usc.user_id, usc.changed_at DESC ), + -- status_changes_during_range defines the status of each user during the start_time and end_time. + -- If a user is deleted during the time range, we count status changes between the start_time and the deletion date. + -- Theoretically, it should probably not be possible to update the status of a deleted user, but we + -- need to ensure that this is enforced, so that a change in business logic later does not break this graph. status_changes_during_range AS ( SELECT usc.user_id, @@ -3139,6 +3148,8 @@ status_changes_during_range AS ( AND usc.changed_at <= $2::timestamptz AND (ud.user_id IS NULL OR usc.changed_at < ud.deleted_at) ), + -- relevant_status_changes defines the status of each user at any point in time. + -- It includes the status of each user before the start_time, and the status of each user during the start_time and end_time. relevant_status_changes AS ( SELECT user_id, @@ -3154,9 +3165,14 @@ relevant_status_changes AS ( changed_at FROM status_changes_during_range ), + -- statuses defines all the distinct statuses that were present just before and during the time range. + -- This is used to ensure that we have a series for every relevant status. statuses AS ( SELECT DISTINCT new_status FROM relevant_status_changes ), + -- We only want to count the latest status change for each user on each date and then filter them by the relevant status. + -- We use the row_number function to ensure that we only count the latest status change for each user on each date. + -- We then filter the status changes by the relevant status in the final select statement below. ranked_status_change_per_user_per_date AS ( SELECT d.date, @@ -3210,22 +3226,6 @@ type GetUserStatusCountsOverTimeRow struct { // Accumulation: // We do not start counting from 0 at the start_time. We check the last status change before the start_time for each user. As such, // the result shows the total number of users in each status on any particular day. -// dates_of_interest defines all points in time that are relevant to the query. -// It includes the start_time, all status changes, all deletions, and the end_time. -// latest_status_before_range defines the status of each user before the start_time. -// We do not include users who were deleted before the start_time. We use this to ensure that -// we correctly count users prior to the start_time for a complete graph. -// status_changes_during_range defines the status of each user during the start_time and end_time. -// If a user is deleted during the time range, we count status changes prior to the deletion. -// Theoretically, it should probably not be possible to update the status of a deleted user, but we -// need to ensure that this is enforced, so that a change in business logic later does not break this graph. -// relevant_status_changes defines the status of each user at any point in time. -// It includes the status of each user before the start_time, and the status of each user during the start_time and end_time. -// statuses defines all the distinct statuses that were present just before and during the time range. -// This is used to ensure that we have a series for every relevant status. -// We only want to count the latest status change for each user on each date and then filter them by the relevant status. -// We use the row_number function to ensure that we only count the latest status change for each user on each date. -// We then filter the status changes by the relevant status in the final select statement below. func (q *sqlQuerier) GetUserStatusCountsOverTime(ctx context.Context, arg GetUserStatusCountsOverTimeParams) ([]GetUserStatusCountsOverTimeRow, error) { rows, err := q.db.QueryContext(ctx, getUserStatusCountsOverTime, arg.StartTime, arg.EndTime) if err != nil { diff --git a/coderd/database/queries/insights.sql b/coderd/database/queries/insights.sql index d599e2989a56e..edf20763f046b 100644 --- a/coderd/database/queries/insights.sql +++ b/coderd/database/queries/insights.sql @@ -786,8 +786,8 @@ GROUP BY utp.num, utp.template_ids, utp.name, utp.type, utp.display_name, utp.de -- We do not start counting from 0 at the start_time. We check the last status change before the start_time for each user. As such, -- the result shows the total number of users in each status on any particular day. WITH --- dates_of_interest defines all points in time that are relevant to the query. --- It includes the start_time, all status changes, all deletions, and the end_time. + -- dates_of_interest defines all points in time that are relevant to the query. + -- It includes the start_time, all status changes, all deletions, and the end_time. dates_of_interest AS ( SELECT @start_time::timestamptz AS date @@ -809,9 +809,9 @@ dates_of_interest AS ( SELECT @end_time::timestamptz AS date ), --- latest_status_before_range defines the status of each user before the start_time. --- We do not include users who were deleted before the start_time. We use this to ensure that --- we correctly count users prior to the start_time for a complete graph. + -- latest_status_before_range defines the status of each user before the start_time. + -- We do not include users who were deleted before the start_time. We use this to ensure that + -- we correctly count users prior to the start_time for a complete graph. latest_status_before_range AS ( SELECT DISTINCT usc.user_id, @@ -823,10 +823,10 @@ latest_status_before_range AS ( AND (ud.user_id IS NULL OR ud.deleted_at > @start_time::timestamptz) ORDER BY usc.user_id, usc.changed_at DESC ), --- status_changes_during_range defines the status of each user during the start_time and end_time. --- If a user is deleted during the time range, we count status changes prior to the deletion. --- Theoretically, it should probably not be possible to update the status of a deleted user, but we --- need to ensure that this is enforced, so that a change in business logic later does not break this graph. + -- status_changes_during_range defines the status of each user during the start_time and end_time. + -- If a user is deleted during the time range, we count status changes between the start_time and the deletion date. + -- Theoretically, it should probably not be possible to update the status of a deleted user, but we + -- need to ensure that this is enforced, so that a change in business logic later does not break this graph. status_changes_during_range AS ( SELECT usc.user_id, @@ -838,8 +838,8 @@ status_changes_during_range AS ( AND usc.changed_at <= @end_time::timestamptz AND (ud.user_id IS NULL OR usc.changed_at < ud.deleted_at) ), --- relevant_status_changes defines the status of each user at any point in time. --- It includes the status of each user before the start_time, and the status of each user during the start_time and end_time. + -- relevant_status_changes defines the status of each user at any point in time. + -- It includes the status of each user before the start_time, and the status of each user during the start_time and end_time. relevant_status_changes AS ( SELECT user_id, @@ -855,14 +855,14 @@ relevant_status_changes AS ( changed_at FROM status_changes_during_range ), --- statuses defines all the distinct statuses that were present just before and during the time range. --- This is used to ensure that we have a series for every relevant status. + -- statuses defines all the distinct statuses that were present just before and during the time range. + -- This is used to ensure that we have a series for every relevant status. statuses AS ( SELECT DISTINCT new_status FROM relevant_status_changes ), --- We only want to count the latest status change for each user on each date and then filter them by the relevant status. --- We use the row_number function to ensure that we only count the latest status change for each user on each date. --- We then filter the status changes by the relevant status in the final select statement below. + -- We only want to count the latest status change for each user on each date and then filter them by the relevant status. + -- We use the row_number function to ensure that we only count the latest status change for each user on each date. + -- We then filter the status changes by the relevant status in the final select statement below. ranked_status_change_per_user_per_date AS ( SELECT d.date, From 4de334fa72166f06231d1daba4579367d27c7448 Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Thu, 9 Jan 2025 10:26:53 +0000 Subject: [PATCH 23/32] review notes --- coderd/database/querier_test.go | 4 ---- coderd/database/queries.sql.go | 2 +- coderd/database/queries/insights.sql | 2 +- coderd/insights.go | 7 ++++--- 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index 8d0d7edf312f6..3d512a890e708 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -2321,10 +2321,6 @@ func TestGetUserStatusCountsOverTime(t *testing.T) { name: "Suspended Only", status: database.UserStatusSuspended, }, - // { - // name: "Deleted Only", - // status: database.UserStatusDeleted, - // }, } for _, tc := range testCases { diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 43adacb2ceb75..0b702ab42306c 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3183,7 +3183,7 @@ ranked_status_change_per_user_per_date AS ( LEFT JOIN relevant_status_changes rsc1 ON rsc1.changed_at <= d.date ) SELECT - date, + rscpupd.date, statuses.new_status AS status, COUNT(rscpupd.user_id) FILTER ( WHERE rscpupd.rn = 1 diff --git a/coderd/database/queries/insights.sql b/coderd/database/queries/insights.sql index edf20763f046b..8bd7df5c3eb72 100644 --- a/coderd/database/queries/insights.sql +++ b/coderd/database/queries/insights.sql @@ -873,7 +873,7 @@ ranked_status_change_per_user_per_date AS ( LEFT JOIN relevant_status_changes rsc1 ON rsc1.changed_at <= d.date ) SELECT - date, + rscpupd.date, statuses.new_status AS status, COUNT(rscpupd.user_id) FILTER ( WHERE rscpupd.rn = 1 diff --git a/coderd/insights.go b/coderd/insights.go index 10205439e2b4a..50691c62514be 100644 --- a/coderd/insights.go +++ b/coderd/insights.go @@ -329,6 +329,10 @@ func (api *API) insightsUserStatusCountsOverTime(rw http.ResponseWriter, r *http EndTime: nextHourInLoc, }) if err != nil { + if httpapi.IsUnauthorizedError(err) { + httpapi.Forbidden(rw) + return + } httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error fetching user status counts over time.", Detail: err.Error(), @@ -342,9 +346,6 @@ func (api *API) insightsUserStatusCountsOverTime(rw http.ResponseWriter, r *http for _, row := range rows { status := codersdk.UserStatus(row.Status) - if _, ok := resp.StatusCounts[status]; !ok { - resp.StatusCounts[status] = make([]codersdk.UserStatusChangeCount, 0) - } resp.StatusCounts[status] = append(resp.StatusCounts[status], codersdk.UserStatusChangeCount{ Date: row.Date, Count: row.Count, From 8fca0c5a01dcb7f1c38758fe9c15d7f6870063cb Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Thu, 9 Jan 2025 10:41:27 +0000 Subject: [PATCH 24/32] make gen --- ...ql => 000283_user_status_changes.down.sql} | 0 ....sql => 000283_user_status_changes.up.sql} | 0 ....sql => 000283_user_status_changes.up.sql} | 0 docs/reference/api/insights.md | 32 ++++++++--------- docs/reference/api/schemas.md | 36 +++++++++---------- site/src/api/typesGenerated.ts | 18 +++------- 6 files changed, 38 insertions(+), 48 deletions(-) rename coderd/database/migrations/{000282_user_status_changes.down.sql => 000283_user_status_changes.down.sql} (100%) rename coderd/database/migrations/{000282_user_status_changes.up.sql => 000283_user_status_changes.up.sql} (100%) rename coderd/database/migrations/testdata/fixtures/{000282_user_status_changes.up.sql => 000283_user_status_changes.up.sql} (100%) diff --git a/coderd/database/migrations/000282_user_status_changes.down.sql b/coderd/database/migrations/000283_user_status_changes.down.sql similarity index 100% rename from coderd/database/migrations/000282_user_status_changes.down.sql rename to coderd/database/migrations/000283_user_status_changes.down.sql diff --git a/coderd/database/migrations/000282_user_status_changes.up.sql b/coderd/database/migrations/000283_user_status_changes.up.sql similarity index 100% rename from coderd/database/migrations/000282_user_status_changes.up.sql rename to coderd/database/migrations/000283_user_status_changes.up.sql diff --git a/coderd/database/migrations/testdata/fixtures/000282_user_status_changes.up.sql b/coderd/database/migrations/testdata/fixtures/000283_user_status_changes.up.sql similarity index 100% rename from coderd/database/migrations/testdata/fixtures/000282_user_status_changes.up.sql rename to coderd/database/migrations/testdata/fixtures/000283_user_status_changes.up.sql diff --git a/docs/reference/api/insights.md b/docs/reference/api/insights.md index 1d37811a6784d..c092411c76bcd 100644 --- a/docs/reference/api/insights.md +++ b/docs/reference/api/insights.md @@ -277,7 +277,7 @@ curl -X GET http://coder-server:8080/api/v2/insights/user-status-counts-over-tim ### Parameters | Name | In | Type | Required | Description | -| ----------- | ----- | ------- | -------- | -------------------------- | +|-------------|-------|---------|----------|----------------------------| | `tz_offset` | query | integer | true | Time-zone offset (e.g. -2) | ### Example responses @@ -286,27 +286,27 @@ curl -X GET http://coder-server:8080/api/v2/insights/user-status-counts-over-tim ```json { - "status_counts": { - "property1": [ - { - "count": 10, - "date": "2019-08-24T14:15:22Z" - } - ], - "property2": [ - { - "count": 10, - "date": "2019-08-24T14:15:22Z" - } - ] - } + "status_counts": { + "property1": [ + { + "count": 10, + "date": "2019-08-24T14:15:22Z" + } + ], + "property2": [ + { + "count": 10, + "date": "2019-08-24T14:15:22Z" + } + ] + } } ``` ### Responses | Status | Meaning | Description | Schema | -| ------ | ------------------------------------------------------- | ----------- | ------------------------------------------------------------------------------------------------------ | +|--------|---------------------------------------------------------|-------------|--------------------------------------------------------------------------------------------------------| | 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | [codersdk.GetUserStatusCountsOverTimeResponse](schemas.md#codersdkgetuserstatuscountsovertimeresponse) | To perform this operation, you must be authenticated. [Learn more](authentication.md). diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index 6c9a9f64bc7da..0837a5348e977 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -3004,27 +3004,27 @@ Git clone makes use of this by parsing the URL from: 'Username for "https://gith ```json { - "status_counts": { - "property1": [ - { - "count": 10, - "date": "2019-08-24T14:15:22Z" - } - ], - "property2": [ - { - "count": 10, - "date": "2019-08-24T14:15:22Z" - } - ] - } + "status_counts": { + "property1": [ + { + "count": 10, + "date": "2019-08-24T14:15:22Z" + } + ], + "property2": [ + { + "count": 10, + "date": "2019-08-24T14:15:22Z" + } + ] + } } ``` ### Properties | Name | Type | Required | Restrictions | Description | -| ------------------ | ------------------------------------------------------------------------- | -------- | ------------ | ----------- | +|--------------------|---------------------------------------------------------------------------|----------|--------------|-------------| | `status_counts` | object | false | | | | » `[any property]` | array of [codersdk.UserStatusChangeCount](#codersdkuserstatuschangecount) | false | | | @@ -6756,15 +6756,15 @@ If the schedule is empty, the user will be updated to use the default schedule.| ```json { - "count": 10, - "date": "2019-08-24T14:15:22Z" + "count": 10, + "date": "2019-08-24T14:15:22Z" } ``` ### Properties | Name | Type | Required | Restrictions | Description | -| ------- | ------- | -------- | ------------ | ----------- | +|---------|---------|----------|--------------|-------------| | `count` | integer | false | | | | `date` | string | false | | | diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index d0ffc1aade125..92b167c958894 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -884,22 +884,12 @@ export interface GenerateAPIKeyResponse { // From codersdk/insights.go export interface GetUserStatusCountsOverTimeRequest { - readonly offset: string; + readonly offset: string; } // From codersdk/insights.go export interface GetUserStatusCountsOverTimeResponse { - readonly status_counts: Record; -} - -// From codersdk/insights.go -export interface GetUserStatusCountsOverTimeRequest { - readonly offset: string; -} - -// From codersdk/insights.go -export interface GetUserStatusCountsOverTimeResponse { - readonly status_counts: Record; + readonly status_counts: Record; } // From codersdk/users.go @@ -2712,8 +2702,8 @@ export type UserStatus = "active" | "dormant" | "suspended"; // From codersdk/insights.go export interface UserStatusChangeCount { - readonly date: string; - readonly count: number; + readonly date: string; + readonly count: number; } export const UserStatuses: UserStatus[] = ["active", "dormant", "suspended"]; From b06179cbe500fdab08315ae510429b52f7bb44ce Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Thu, 9 Jan 2025 10:50:39 +0000 Subject: [PATCH 25/32] remove frontend changes --- site/src/api/api.ts | 13 -- site/src/api/queries/insights.ts | 7 - site/src/api/queries/users.ts | 1 + site/src/api/typesGenerated.ts | 16 -- .../ActiveUserChart.stories.tsx | 69 +------ .../ActiveUserChart/ActiveUserChart.tsx | 121 +++--------- .../GeneralSettingsPage.tsx | 7 +- .../GeneralSettingsPageView.stories.tsx | 174 +++++++----------- .../GeneralSettingsPageView.tsx | 82 +++++---- .../TemplateInsightsPage.tsx | 12 +- 10 files changed, 148 insertions(+), 354 deletions(-) diff --git a/site/src/api/api.ts b/site/src/api/api.ts index 74ca5ce685007..6b0e685b177eb 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -2097,19 +2097,6 @@ class ApiMethods { return response.data; }; - getInsightsUserStatusCountsOverTime = async ( - offset = Math.trunc(new Date().getTimezoneOffset() / 60), - ): Promise => { - const searchParams = new URLSearchParams({ - offset: offset.toString(), - }); - const response = await this.axios.get( - `/api/v2/insights/user-status-counts-over-time?${searchParams}`, - ); - - return response.data; - }; - getHealth = async (force = false) => { const params = new URLSearchParams({ force: force.toString() }); const response = await this.axios.get( diff --git a/site/src/api/queries/insights.ts b/site/src/api/queries/insights.ts index 8f56b5982cd84..a7044a2f2469f 100644 --- a/site/src/api/queries/insights.ts +++ b/site/src/api/queries/insights.ts @@ -20,10 +20,3 @@ export const insightsUserActivity = (params: InsightsParams) => { queryFn: () => API.getInsightsUserActivity(params), }; }; - -export const userStatusCountsOverTime = () => { - return { - queryKey: ["userStatusCountsOverTime"], - queryFn: () => API.getInsightsUserStatusCountsOverTime(), - }; -}; diff --git a/site/src/api/queries/users.ts b/site/src/api/queries/users.ts index 833d88e6baeef..77d879abe3258 100644 --- a/site/src/api/queries/users.ts +++ b/site/src/api/queries/users.ts @@ -9,6 +9,7 @@ import type { UpdateUserProfileRequest, User, UsersRequest, + ValidateUserPasswordRequest, } from "api/typesGenerated"; import { type MetadataState, diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 92b167c958894..4956de8691ed7 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -882,16 +882,6 @@ export interface GenerateAPIKeyResponse { readonly key: string; } -// From codersdk/insights.go -export interface GetUserStatusCountsOverTimeRequest { - readonly offset: string; -} - -// From codersdk/insights.go -export interface GetUserStatusCountsOverTimeResponse { - readonly status_counts: Record; -} - // From codersdk/users.go export interface GetUsersResponse { readonly users: readonly User[]; @@ -2700,12 +2690,6 @@ export interface UserRoles { // From codersdk/users.go export type UserStatus = "active" | "dormant" | "suspended"; -// From codersdk/insights.go -export interface UserStatusChangeCount { - readonly date: string; - readonly count: number; -} - export const UserStatuses: UserStatus[] = ["active", "dormant", "suspended"]; // From codersdk/users.go diff --git a/site/src/components/ActiveUserChart/ActiveUserChart.stories.tsx b/site/src/components/ActiveUserChart/ActiveUserChart.stories.tsx index b77886b63fd2a..4f28d7243a0bf 100644 --- a/site/src/components/ActiveUserChart/ActiveUserChart.stories.tsx +++ b/site/src/components/ActiveUserChart/ActiveUserChart.stories.tsx @@ -5,19 +5,14 @@ const meta: Meta = { title: "components/ActiveUserChart", component: ActiveUserChart, args: { - series: [ - { - label: "Daily", - data: [ - { date: "1/1/2024", amount: 5 }, - { date: "1/2/2024", amount: 6 }, - { date: "1/3/2024", amount: 7 }, - { date: "1/4/2024", amount: 8 }, - { date: "1/5/2024", amount: 9 }, - { date: "1/6/2024", amount: 10 }, - { date: "1/7/2024", amount: 11 }, - ], - }, + data: [ + { date: "1/1/2024", amount: 5 }, + { date: "1/2/2024", amount: 6 }, + { date: "1/3/2024", amount: 7 }, + { date: "1/4/2024", amount: 8 }, + { date: "1/5/2024", amount: 9 }, + { date: "1/6/2024", amount: 10 }, + { date: "1/7/2024", amount: 11 }, ], interval: "day", }, @@ -27,51 +22,3 @@ export default meta; type Story = StoryObj; export const Example: Story = {}; - -export const MultipleSeries: Story = { - args: { - series: [ - { - label: "Active", - data: [ - { date: "1/1/2024", amount: 150 }, - { date: "1/2/2024", amount: 165 }, - { date: "1/3/2024", amount: 180 }, - { date: "1/4/2024", amount: 155 }, - { date: "1/5/2024", amount: 190 }, - { date: "1/6/2024", amount: 200 }, - { date: "1/7/2024", amount: 210 }, - ], - color: "green", - }, - { - label: "Dormant", - data: [ - { date: "1/1/2024", amount: 80 }, - { date: "1/2/2024", amount: 82 }, - { date: "1/3/2024", amount: 85 }, - { date: "1/4/2024", amount: 88 }, - { date: "1/5/2024", amount: 90 }, - { date: "1/6/2024", amount: 92 }, - { date: "1/7/2024", amount: 95 }, - ], - color: "grey", - }, - { - label: "Suspended", - data: [ - { date: "1/1/2024", amount: 20 }, - { date: "1/2/2024", amount: 22 }, - { date: "1/3/2024", amount: 25 }, - { date: "1/4/2024", amount: 23 }, - { date: "1/5/2024", amount: 28 }, - { date: "1/6/2024", amount: 30 }, - { date: "1/7/2024", amount: 32 }, - ], - color: "red", - }, - ], - interval: "day", - userLimit: 100, - }, -}; diff --git a/site/src/components/ActiveUserChart/ActiveUserChart.tsx b/site/src/components/ActiveUserChart/ActiveUserChart.tsx index 10acb6ec9fc90..41345ea8f03f8 100644 --- a/site/src/components/ActiveUserChart/ActiveUserChart.tsx +++ b/site/src/components/ActiveUserChart/ActiveUserChart.tsx @@ -1,7 +1,5 @@ import "chartjs-adapter-date-fns"; import { useTheme } from "@emotion/react"; -import LaunchOutlined from "@mui/icons-material/LaunchOutlined"; -import Button from "@mui/material/Button"; import { CategoryScale, Chart as ChartJS, @@ -16,7 +14,6 @@ import { Tooltip, defaults, } from "chart.js"; -import annotationPlugin from "chartjs-plugin-annotation"; import { HelpTooltip, HelpTooltipContent, @@ -38,51 +35,32 @@ ChartJS.register( Title, Tooltip, Legend, - annotationPlugin, ); -export interface DataSeries { - label?: string; - data: readonly { date: string; amount: number }[]; - color?: string; // Optional custom color -} - export interface ActiveUserChartProps { - series: DataSeries[]; - userLimit?: number; + data: readonly { date: string; amount: number }[]; interval: "day" | "week"; } export const ActiveUserChart: FC = ({ - series, - userLimit, + data, interval, }) => { const theme = useTheme(); + const labels = data.map((val) => dayjs(val.date).format("YYYY-MM-DD")); + const chartData = data.map((val) => val.amount); + defaults.font.family = theme.typography.fontFamily as string; defaults.color = theme.palette.text.secondary; const options: ChartOptions<"line"> = { responsive: true, animation: false, - interaction: { - mode: "index", - }, plugins: { - legend: - series.length > 1 - ? { - display: false, - position: "top" as const, - labels: { - usePointStyle: true, - pointStyle: "line", - }, - } - : { - display: false, - }, + legend: { + display: false, + }, tooltip: { displayColors: false, callbacks: { @@ -92,24 +70,6 @@ export const ActiveUserChart: FC = ({ }, }, }, - annotation: { - annotations: [ - { - type: "line", - scaleID: "y", - value: userLimit, - borderColor: "white", - borderWidth: 2, - label: { - content: "Active User limit", - color: theme.palette.primary.contrastText, - display: true, - textStrokeWidth: 2, - textStrokeColor: theme.palette.background.paper, - }, - }, - ], - }, }, scales: { y: { @@ -118,12 +78,11 @@ export const ActiveUserChart: FC = ({ ticks: { precision: 0, }, - stacked: true, }, x: { grid: { color: theme.palette.divider }, ticks: { - stepSize: series[0].data.length > 10 ? 2 : undefined, + stepSize: data.length > 10 ? 2 : undefined, }, type: "time", time: { @@ -138,16 +97,16 @@ export const ActiveUserChart: FC = ({ - dayjs(val.date).format("YYYY-MM-DD"), - ), - datasets: series.map((s) => ({ - label: s.label, - data: s.data.map((val) => val.amount), - pointBackgroundColor: s.color || theme.roles.active.outline, - pointBorderColor: s.color || theme.roles.active.outline, - borderColor: s.color || theme.roles.active.outline, - })), + labels: labels, + datasets: [ + { + label: `${interval === "day" ? "Daily" : "Weekly"} Active Users`, + data: chartData, + pointBackgroundColor: theme.roles.active.outline, + pointBorderColor: theme.roles.active.outline, + borderColor: theme.roles.active.outline, + }, + ], }} options={options} /> @@ -161,13 +120,11 @@ type ActiveUsersTitleProps = { export const ActiveUsersTitle: FC = ({ interval }) => { return (
- {interval === "day" ? "Daily" : "Weekly"} User Activity + {interval === "day" ? "Daily" : "Weekly"} Active Users - - How do we calculate user activity? - + How do we calculate active users? When a connection is initiated to a user's workspace they are considered an active user. e.g. apps, web terminal, SSH. This is for @@ -179,39 +136,3 @@ export const ActiveUsersTitle: FC = ({ interval }) => {
); }; - -export type UserStatusTitleProps = { - interval: "day" | "week"; -}; - -export const UserStatusTitle: FC = ({ interval }) => { - return ( -
- {interval === "day" ? "Daily" : "Weekly"} User Status - - - - What are user statuses? - - - Active users count towards your license consumption. Dormant or - suspended users do not. Any user who has logged into the coder - platform within the last 90 days is considered active. - - - - - -
- ); -}; diff --git a/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx b/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx index 3de614a42ac39..2b094cbf89b26 100644 --- a/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx +++ b/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx @@ -1,6 +1,6 @@ import { deploymentDAUs } from "api/queries/deployment"; +import { entitlements } from "api/queries/entitlements"; import { availableExperiments, experiments } from "api/queries/experiments"; -import { userStatusCountsOverTime } from "api/queries/insights"; import { useEmbeddedMetadata } from "hooks/useEmbeddedMetadata"; import { useDeploymentSettings } from "modules/management/DeploymentSettingsProvider"; import type { FC } from "react"; @@ -15,8 +15,9 @@ const GeneralSettingsPage: FC = () => { const safeExperimentsQuery = useQuery(availableExperiments()); const { metadata } = useEmbeddedMetadata(); + const entitlementsQuery = useQuery(entitlements(metadata.entitlements)); const enabledExperimentsQuery = useQuery(experiments(metadata.experiments)); - const userStatusCountsOverTimeQuery = useQuery(userStatusCountsOverTime()); + const safeExperiments = safeExperimentsQuery.data?.safe ?? []; const invalidExperiments = enabledExperimentsQuery.data?.filter((exp) => { @@ -32,9 +33,9 @@ const GeneralSettingsPage: FC = () => { deploymentOptions={deploymentConfig.options} deploymentDAUs={deploymentDAUsQuery.data} deploymentDAUsError={deploymentDAUsQuery.error} + entitlements={entitlementsQuery.data} invalidExperiments={invalidExperiments} safeExperiments={safeExperiments} - userStatusCountsOverTime={userStatusCountsOverTimeQuery.data} /> ); diff --git a/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx b/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx index 78291ee03b4d8..05ed426d5dcc9 100644 --- a/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx +++ b/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx @@ -1,5 +1,9 @@ import type { Meta, StoryObj } from "@storybook/react"; -import { MockDeploymentDAUResponse, mockApiError } from "testHelpers/entities"; +import { + MockDeploymentDAUResponse, + MockEntitlementsWithUserLimit, + mockApiError, +} from "testHelpers/entities"; import { GeneralSettingsPageView } from "./GeneralSettingsPageView"; const meta: Meta = { @@ -38,100 +42,7 @@ const meta: Meta = { deploymentDAUs: MockDeploymentDAUResponse, invalidExperiments: [], safeExperiments: [], - userStatusCountsOverTime: { - status_counts: { - active: [ - { - date: "1/1/2024", - count: 1, - }, - { - date: "1/2/2024", - count: 8, - }, - { - date: "1/3/2024", - count: 8, - }, - { - date: "1/4/2024", - count: 6, - }, - { - date: "1/5/2024", - count: 6, - }, - { - date: "1/6/2024", - count: 6, - }, - { - date: "1/7/2024", - count: 6, - }, - ], - dormant: [ - { - date: "1/1/2024", - count: 0, - }, - { - date: "1/2/2024", - count: 3, - }, - { - date: "1/3/2024", - count: 3, - }, - { - date: "1/4/2024", - count: 3, - }, - { - date: "1/5/2024", - count: 3, - }, - { - date: "1/6/2024", - count: 3, - }, - { - date: "1/7/2024", - count: 3, - }, - ], - suspended: [ - { - date: "1/1/2024", - count: 0, - }, - { - date: "1/2/2024", - count: 0, - }, - { - date: "1/3/2024", - count: 0, - }, - { - date: "1/4/2024", - count: 2, - }, - { - date: "1/5/2024", - count: 2, - }, - { - date: "1/6/2024", - count: 2, - }, - { - date: "1/7/2024", - count: 2, - }, - ], - }, - }, + entitlements: undefined, }, }; @@ -227,26 +138,73 @@ export const invalidExperimentsEnabled: Story = { }, }; -export const UnlicensedInstallation: Story = { - args: {}, -}; - -export const LicensedWithNoUserLimit: Story = { - args: {}, +export const WithLicenseUtilization: Story = { + args: { + entitlements: { + ...MockEntitlementsWithUserLimit, + features: { + ...MockEntitlementsWithUserLimit.features, + user_limit: { + ...MockEntitlementsWithUserLimit.features.user_limit, + enabled: true, + actual: 75, + limit: 100, + entitlement: "entitled", + }, + }, + }, + }, }; -export const LicensedWithPlentyOfSpareLicenses: Story = { +export const HighLicenseUtilization: Story = { args: { - activeUserLimit: 100, + entitlements: { + ...MockEntitlementsWithUserLimit, + features: { + ...MockEntitlementsWithUserLimit.features, + user_limit: { + ...MockEntitlementsWithUserLimit.features.user_limit, + enabled: true, + actual: 95, + limit: 100, + entitlement: "entitled", + }, + }, + }, }, }; -export const TotalUsersExceedsLicenseButNotActiveUsers: Story = { +export const ExceedsLicenseUtilization: Story = { args: { - activeUserLimit: 8, + entitlements: { + ...MockEntitlementsWithUserLimit, + features: { + ...MockEntitlementsWithUserLimit.features, + user_limit: { + ...MockEntitlementsWithUserLimit.features.user_limit, + enabled: true, + actual: 100, + limit: 95, + entitlement: "entitled", + }, + }, + }, }, }; - -export const ManyUsers: Story = { - args: {}, +export const NoLicenseLimit: Story = { + args: { + entitlements: { + ...MockEntitlementsWithUserLimit, + features: { + ...MockEntitlementsWithUserLimit.features, + user_limit: { + ...MockEntitlementsWithUserLimit.features.user_limit, + enabled: false, + actual: 0, + limit: 0, + entitlement: "entitled", + }, + }, + }, + }, }; diff --git a/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPageView.tsx b/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPageView.tsx index 3680e823516a6..df5550d70e965 100644 --- a/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPageView.tsx +++ b/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPageView.tsx @@ -1,15 +1,14 @@ import AlertTitle from "@mui/material/AlertTitle"; +import LinearProgress from "@mui/material/LinearProgress"; import type { DAUsResponse, + Entitlements, Experiments, - GetUserStatusCountsOverTimeResponse, SerpentOption, } from "api/typesGenerated"; import { ActiveUserChart, ActiveUsersTitle, - type DataSeries, - UserStatusTitle, } from "components/ActiveUserChart/ActiveUserChart"; import { ErrorAlert } from "components/Alert/ErrorAlert"; import { SettingsHeader } from "components/SettingsHeader/SettingsHeader"; @@ -25,8 +24,7 @@ export type GeneralSettingsPageViewProps = { deploymentOptions: SerpentOption[]; deploymentDAUs?: DAUsResponse; deploymentDAUsError: unknown; - userStatusCountsOverTime?: GetUserStatusCountsOverTimeResponse; - activeUserLimit?: number; + entitlements: Entitlements | undefined; readonly invalidExperiments: Experiments | string[]; readonly safeExperiments: Experiments | string[]; }; @@ -35,29 +33,16 @@ export const GeneralSettingsPageView: FC = ({ deploymentOptions, deploymentDAUs, deploymentDAUsError, - userStatusCountsOverTime, - activeUserLimit, + entitlements, safeExperiments, invalidExperiments, }) => { - const colors: Record = { - active: "green", - dormant: "grey", - deleted: "red", - }; - let series: DataSeries[] = []; - if (userStatusCountsOverTime?.status_counts) { - series = Object.entries(userStatusCountsOverTime.status_counts).map( - ([status, counts]) => ({ - label: status, - data: counts.map((count) => ({ - date: count.date.toString(), - amount: count.count, - })), - color: colors[status], - }), - ); - } + const licenseUtilizationPercentage = + entitlements?.features?.user_limit?.actual && + entitlements?.features?.user_limit?.limit + ? entitlements.features.user_limit.actual / + entitlements.features.user_limit.limit + : undefined; return ( <> = ({ {Boolean(deploymentDAUsError) && ( )} - {series.length && ( - }> - - - )} {deploymentDAUs && ( - }> - + }> + + + + )} + {licenseUtilizationPercentage && ( + + + + {Math.round(licenseUtilizationPercentage * 100)}% used ( + {entitlements!.features.user_limit.actual}/ + {entitlements!.features.user_limit.limit} users) + )} {invalidExperiments.length > 0 && ( diff --git a/site/src/pages/TemplatePage/TemplateInsightsPage/TemplateInsightsPage.tsx b/site/src/pages/TemplatePage/TemplateInsightsPage/TemplateInsightsPage.tsx index 2b873c325e274..097b8fce513e7 100644 --- a/site/src/pages/TemplatePage/TemplateInsightsPage/TemplateInsightsPage.tsx +++ b/site/src/pages/TemplatePage/TemplateInsightsPage/TemplateInsightsPage.tsx @@ -258,14 +258,10 @@ const ActiveUsersPanel: FC = ({ {data && data.length > 0 && ( ({ - amount: d.active_users, - date: d.start_time, - })), - }, - ]} + data={data.map((d) => ({ + amount: d.active_users, + date: d.start_time, + }))} /> )} From 213b288d0e9c41202e7342e2ecc9e30453caa07a Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Thu, 9 Jan 2025 11:02:19 +0000 Subject: [PATCH 26/32] rename GetUserStatusCountsOverTime to GetUserStatusCounts --- coderd/apidoc/docs.go | 8 ++++---- coderd/apidoc/swagger.json | 8 ++++---- coderd/coderd.go | 2 +- coderd/database/dbauthz/dbauthz.go | 4 ++-- coderd/database/dbauthz/dbauthz_test.go | 4 ++-- coderd/database/dbmem/dbmem.go | 8 ++++---- coderd/database/dbmetrics/querymetrics.go | 6 +++--- coderd/database/dbmock/dbmock.go | 14 +++++++------- coderd/database/querier.go | 4 ++-- coderd/database/querier_test.go | 16 ++++++++-------- coderd/database/queries.sql.go | 16 ++++++++-------- coderd/database/queries/insights.sql | 4 ++-- coderd/insights.go | 12 ++++++------ codersdk/insights.go | 14 +++++++------- docs/reference/api/insights.md | 10 +++++----- docs/reference/api/schemas.md | 2 +- site/src/api/typesGenerated.ts | 16 ++++++++++++++++ 17 files changed, 82 insertions(+), 66 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 0243e8310c066..9fb17a59f43e5 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -1398,7 +1398,7 @@ const docTemplate = `{ } } }, - "/insights/user-status-counts-over-time": { + "/insights/user-status-counts": { "get": { "security": [ { @@ -1412,7 +1412,7 @@ const docTemplate = `{ "Insights" ], "summary": "Get insights about user status counts over time", - "operationId": "get-insights-about-user-status-counts-over-time", + "operationId": "get-insights-about-user-status-counts", "parameters": [ { "type": "integer", @@ -1426,7 +1426,7 @@ const docTemplate = `{ "200": { "description": "OK", "schema": { - "$ref": "#/definitions/codersdk.GetUserStatusCountsOverTimeResponse" + "$ref": "#/definitions/codersdk.GetUserStatusCountsResponse" } } } @@ -11241,7 +11241,7 @@ const docTemplate = `{ } } }, - "codersdk.GetUserStatusCountsOverTimeResponse": { + "codersdk.GetUserStatusCountsResponse": { "type": "object", "properties": { "status_counts": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 5d9dda0664489..ca2f869336a4d 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -1219,7 +1219,7 @@ } } }, - "/insights/user-status-counts-over-time": { + "/insights/user-status-counts": { "get": { "security": [ { @@ -1229,7 +1229,7 @@ "produces": ["application/json"], "tags": ["Insights"], "summary": "Get insights about user status counts over time", - "operationId": "get-insights-about-user-status-counts-over-time", + "operationId": "get-insights-about-user-status-counts", "parameters": [ { "type": "integer", @@ -1243,7 +1243,7 @@ "200": { "description": "OK", "schema": { - "$ref": "#/definitions/codersdk.GetUserStatusCountsOverTimeResponse" + "$ref": "#/definitions/codersdk.GetUserStatusCountsResponse" } } } @@ -10084,7 +10084,7 @@ } } }, - "codersdk.GetUserStatusCountsOverTimeResponse": { + "codersdk.GetUserStatusCountsResponse": { "type": "object", "properties": { "status_counts": { diff --git a/coderd/coderd.go b/coderd/coderd.go index c197c08fd5cd9..7b8cde9dc6ae4 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -1281,7 +1281,7 @@ func New(options *Options) *API { r.Use(apiKeyMiddleware) r.Get("/daus", api.deploymentDAUs) r.Get("/user-activity", api.insightsUserActivity) - r.Get("/user-status-counts-over-time", api.insightsUserStatusCountsOverTime) + r.Get("/user-status-counts", api.insightsUserStatusCounts) r.Get("/user-latency", api.insightsUserLatency) r.Get("/templates", api.insightsTemplates) }) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index be04c6170c986..a4c3208aa5e6d 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -2421,11 +2421,11 @@ func (q *querier) GetUserNotificationPreferences(ctx context.Context, userID uui return q.db.GetUserNotificationPreferences(ctx, userID) } -func (q *querier) GetUserStatusCountsOverTime(ctx context.Context, arg database.GetUserStatusCountsOverTimeParams) ([]database.GetUserStatusCountsOverTimeRow, error) { +func (q *querier) GetUserStatusCounts(ctx context.Context, arg database.GetUserStatusCountsParams) ([]database.GetUserStatusCountsRow, error) { if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceUser); err != nil { return nil, err } - return q.db.GetUserStatusCountsOverTime(ctx, arg) + return q.db.GetUserStatusCounts(ctx, arg) } func (q *querier) GetUserWorkspaceBuildParameters(ctx context.Context, params database.GetUserWorkspaceBuildParametersParams) ([]database.GetUserWorkspaceBuildParametersRow, error) { diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 3bc1f80519f0a..78500792933b5 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -1708,8 +1708,8 @@ func (s *MethodTestSuite) TestUser() { rbac.ResourceTemplate.InOrg(orgID), policy.ActionRead, ) })) - s.Run("GetUserStatusCountsOverTime", s.Subtest(func(db database.Store, check *expects) { - check.Args(database.GetUserStatusCountsOverTimeParams{ + s.Run("GetUserStatusCounts", s.Subtest(func(db database.Store, check *expects) { + check.Args(database.GetUserStatusCountsParams{ StartTime: time.Now().Add(-time.Hour * 24 * 30), EndTime: time.Now(), }).Asserts(rbac.ResourceUser, policy.ActionRead) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 736823553af4a..9e3c3621b3420 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -5671,7 +5671,7 @@ func (q *FakeQuerier) GetUserNotificationPreferences(_ context.Context, userID u return out, nil } -func (q *FakeQuerier) GetUserStatusCountsOverTime(_ context.Context, arg database.GetUserStatusCountsOverTimeParams) ([]database.GetUserStatusCountsOverTimeRow, error) { +func (q *FakeQuerier) GetUserStatusCounts(_ context.Context, arg database.GetUserStatusCountsParams) ([]database.GetUserStatusCountsRow, error) { q.mutex.RLock() defer q.mutex.RUnlock() @@ -5680,16 +5680,16 @@ func (q *FakeQuerier) GetUserStatusCountsOverTime(_ context.Context, arg databas return nil, err } - result := make([]database.GetUserStatusCountsOverTimeRow, 0) + result := make([]database.GetUserStatusCountsRow, 0) for _, change := range q.userStatusChanges { if change.ChangedAt.Before(arg.StartTime) || change.ChangedAt.After(arg.EndTime) { continue } date := time.Date(change.ChangedAt.Year(), change.ChangedAt.Month(), change.ChangedAt.Day(), 0, 0, 0, 0, time.UTC) - if !slices.ContainsFunc(result, func(r database.GetUserStatusCountsOverTimeRow) bool { + if !slices.ContainsFunc(result, func(r database.GetUserStatusCountsRow) bool { return r.Status == change.NewStatus && r.Date.Equal(date) }) { - result = append(result, database.GetUserStatusCountsOverTimeRow{ + result = append(result, database.GetUserStatusCountsRow{ Status: change.NewStatus, Date: date, Count: 1, diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index bf5b8954cfeb0..599fad08779ac 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -1344,10 +1344,10 @@ func (m queryMetricsStore) GetUserNotificationPreferences(ctx context.Context, u return r0, r1 } -func (m queryMetricsStore) GetUserStatusCountsOverTime(ctx context.Context, arg database.GetUserStatusCountsOverTimeParams) ([]database.GetUserStatusCountsOverTimeRow, error) { +func (m queryMetricsStore) GetUserStatusCounts(ctx context.Context, arg database.GetUserStatusCountsParams) ([]database.GetUserStatusCountsRow, error) { start := time.Now() - r0, r1 := m.s.GetUserStatusCountsOverTime(ctx, arg) - m.queryLatencies.WithLabelValues("GetUserStatusCountsOverTime").Observe(time.Since(start).Seconds()) + r0, r1 := m.s.GetUserStatusCounts(ctx, arg) + m.queryLatencies.WithLabelValues("GetUserStatusCounts").Observe(time.Since(start).Seconds()) return r0, r1 } diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index 4f743a453dc0d..51d0c59c1d879 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -2825,19 +2825,19 @@ func (mr *MockStoreMockRecorder) GetUserNotificationPreferences(arg0, arg1 any) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserNotificationPreferences", reflect.TypeOf((*MockStore)(nil).GetUserNotificationPreferences), arg0, arg1) } -// GetUserStatusCountsOverTime mocks base method. -func (m *MockStore) GetUserStatusCountsOverTime(arg0 context.Context, arg1 database.GetUserStatusCountsOverTimeParams) ([]database.GetUserStatusCountsOverTimeRow, error) { +// GetUserStatusCounts mocks base method. +func (m *MockStore) GetUserStatusCounts(arg0 context.Context, arg1 database.GetUserStatusCountsParams) ([]database.GetUserStatusCountsRow, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetUserStatusCountsOverTime", arg0, arg1) - ret0, _ := ret[0].([]database.GetUserStatusCountsOverTimeRow) + ret := m.ctrl.Call(m, "GetUserStatusCounts", arg0, arg1) + ret0, _ := ret[0].([]database.GetUserStatusCountsRow) ret1, _ := ret[1].(error) return ret0, ret1 } -// GetUserStatusCountsOverTime indicates an expected call of GetUserStatusCountsOverTime. -func (mr *MockStoreMockRecorder) GetUserStatusCountsOverTime(arg0, arg1 any) *gomock.Call { +// GetUserStatusCounts indicates an expected call of GetUserStatusCounts. +func (mr *MockStoreMockRecorder) GetUserStatusCounts(arg0, arg1 any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserStatusCountsOverTime", reflect.TypeOf((*MockStore)(nil).GetUserStatusCountsOverTime), arg0, arg1) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserStatusCounts", reflect.TypeOf((*MockStore)(nil).GetUserStatusCounts), arg0, arg1) } // GetUserWorkspaceBuildParameters mocks base method. diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 948c5f914ab61..ba151e0e8abb0 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -289,7 +289,7 @@ type sqlcQuerier interface { GetUserLinkByUserIDLoginType(ctx context.Context, arg GetUserLinkByUserIDLoginTypeParams) (UserLink, error) GetUserLinksByUserID(ctx context.Context, userID uuid.UUID) ([]UserLink, error) GetUserNotificationPreferences(ctx context.Context, userID uuid.UUID) ([]NotificationPreference, error) - // GetUserStatusCountsOverTime returns the count of users in each status over time. + // GetUserStatusCounts returns the count of users in each status over time. // The time range is inclusively defined by the start_time and end_time parameters. // // Bucketing: @@ -301,7 +301,7 @@ type sqlcQuerier interface { // Accumulation: // We do not start counting from 0 at the start_time. We check the last status change before the start_time for each user. As such, // the result shows the total number of users in each status on any particular day. - GetUserStatusCountsOverTime(ctx context.Context, arg GetUserStatusCountsOverTimeParams) ([]GetUserStatusCountsOverTimeRow, error) + GetUserStatusCounts(ctx context.Context, arg GetUserStatusCountsParams) ([]GetUserStatusCountsRow, error) GetUserWorkspaceBuildParameters(ctx context.Context, arg GetUserWorkspaceBuildParametersParams) ([]GetUserWorkspaceBuildParametersRow, error) // This will never return deleted users. GetUsers(ctx context.Context, arg GetUsersParams) ([]GetUsersRow, error) diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index 3d512a890e708..0c7a445ed7104 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -2256,7 +2256,7 @@ func TestGroupRemovalTrigger(t *testing.T) { }, db2sdk.List(extraUserGroups, onlyGroupIDs)) } -func TestGetUserStatusCountsOverTime(t *testing.T) { +func TestGetUserStatusCounts(t *testing.T) { t.Parallel() if !dbtestutil.WillUsePostgres() { @@ -2294,7 +2294,7 @@ func TestGetUserStatusCountsOverTime(t *testing.T) { end := dbtime.Now() start := end.Add(-30 * 24 * time.Hour) - counts, err := db.GetUserStatusCountsOverTime(ctx, database.GetUserStatusCountsOverTimeParams{ + counts, err := db.GetUserStatusCounts(ctx, database.GetUserStatusCountsParams{ StartTime: start, EndTime: end, }) @@ -2338,7 +2338,7 @@ func TestGetUserStatusCountsOverTime(t *testing.T) { }) // Query for the last 30 days - userStatusChanges, err := db.GetUserStatusCountsOverTime(ctx, database.GetUserStatusCountsOverTimeParams{ + userStatusChanges, err := db.GetUserStatusCounts(ctx, database.GetUserStatusCountsParams{ StartTime: createdAt, EndTime: today, }) @@ -2506,7 +2506,7 @@ func TestGetUserStatusCountsOverTime(t *testing.T) { require.NoError(t, err) // Query for the last 5 days - userStatusChanges, err := db.GetUserStatusCountsOverTime(ctx, database.GetUserStatusCountsOverTimeParams{ + userStatusChanges, err := db.GetUserStatusCounts(ctx, database.GetUserStatusCountsParams{ StartTime: createdAt, EndTime: today, }) @@ -2635,7 +2635,7 @@ func TestGetUserStatusCountsOverTime(t *testing.T) { }) require.NoError(t, err) - userStatusChanges, err := db.GetUserStatusCountsOverTime(ctx, database.GetUserStatusCountsOverTimeParams{ + userStatusChanges, err := db.GetUserStatusCounts(ctx, database.GetUserStatusCountsParams{ StartTime: createdAt, EndTime: today, }) @@ -2711,7 +2711,7 @@ func TestGetUserStatusCountsOverTime(t *testing.T) { UpdatedAt: createdAt, }) - userStatusChanges, err := db.GetUserStatusCountsOverTime(ctx, database.GetUserStatusCountsOverTimeParams{ + userStatusChanges, err := db.GetUserStatusCounts(ctx, database.GetUserStatusCountsParams{ StartTime: createdAt.Add(time.Hour * 24), EndTime: today, }) @@ -2738,7 +2738,7 @@ func TestGetUserStatusCountsOverTime(t *testing.T) { err = db.UpdateUserDeletedByID(ctx, user.ID) require.NoError(t, err) - userStatusChanges, err := db.GetUserStatusCountsOverTime(ctx, database.GetUserStatusCountsOverTimeParams{ + userStatusChanges, err := db.GetUserStatusCounts(ctx, database.GetUserStatusCountsParams{ StartTime: today.Add(time.Hour * 24), EndTime: today.Add(time.Hour * 48), }) @@ -2760,7 +2760,7 @@ func TestGetUserStatusCountsOverTime(t *testing.T) { err := db.UpdateUserDeletedByID(ctx, user.ID) require.NoError(t, err) - userStatusChanges, err := db.GetUserStatusCountsOverTime(ctx, database.GetUserStatusCountsOverTimeParams{ + userStatusChanges, err := db.GetUserStatusCounts(ctx, database.GetUserStatusCountsParams{ StartTime: createdAt, EndTime: today.Add(time.Hour * 24), }) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 0b702ab42306c..703dbb5fae717 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3094,7 +3094,7 @@ func (q *sqlQuerier) GetUserLatencyInsights(ctx context.Context, arg GetUserLate return items, nil } -const getUserStatusCountsOverTime = `-- name: GetUserStatusCountsOverTime :many +const getUserStatusCounts = `-- name: GetUserStatusCounts :many WITH -- dates_of_interest defines all points in time that are relevant to the query. -- It includes the start_time, all status changes, all deletions, and the end_time. @@ -3203,18 +3203,18 @@ CROSS JOIN statuses GROUP BY date, statuses.new_status ` -type GetUserStatusCountsOverTimeParams struct { +type GetUserStatusCountsParams struct { StartTime time.Time `db:"start_time" json:"start_time"` EndTime time.Time `db:"end_time" json:"end_time"` } -type GetUserStatusCountsOverTimeRow struct { +type GetUserStatusCountsRow struct { Date time.Time `db:"date" json:"date"` Status UserStatus `db:"status" json:"status"` Count int64 `db:"count" json:"count"` } -// GetUserStatusCountsOverTime returns the count of users in each status over time. +// GetUserStatusCounts returns the count of users in each status over time. // The time range is inclusively defined by the start_time and end_time parameters. // // Bucketing: @@ -3226,15 +3226,15 @@ type GetUserStatusCountsOverTimeRow struct { // Accumulation: // We do not start counting from 0 at the start_time. We check the last status change before the start_time for each user. As such, // the result shows the total number of users in each status on any particular day. -func (q *sqlQuerier) GetUserStatusCountsOverTime(ctx context.Context, arg GetUserStatusCountsOverTimeParams) ([]GetUserStatusCountsOverTimeRow, error) { - rows, err := q.db.QueryContext(ctx, getUserStatusCountsOverTime, arg.StartTime, arg.EndTime) +func (q *sqlQuerier) GetUserStatusCounts(ctx context.Context, arg GetUserStatusCountsParams) ([]GetUserStatusCountsRow, error) { + rows, err := q.db.QueryContext(ctx, getUserStatusCounts, arg.StartTime, arg.EndTime) if err != nil { return nil, err } defer rows.Close() - var items []GetUserStatusCountsOverTimeRow + var items []GetUserStatusCountsRow for rows.Next() { - var i GetUserStatusCountsOverTimeRow + var i GetUserStatusCountsRow if err := rows.Scan(&i.Date, &i.Status, &i.Count); err != nil { return nil, err } diff --git a/coderd/database/queries/insights.sql b/coderd/database/queries/insights.sql index 8bd7df5c3eb72..02b454b9c34c9 100644 --- a/coderd/database/queries/insights.sql +++ b/coderd/database/queries/insights.sql @@ -772,8 +772,8 @@ FROM unique_template_params utp JOIN workspace_build_parameters wbp ON (utp.workspace_build_ids @> ARRAY[wbp.workspace_build_id] AND utp.name = wbp.name) GROUP BY utp.num, utp.template_ids, utp.name, utp.type, utp.display_name, utp.description, utp.options, wbp.value; --- name: GetUserStatusCountsOverTime :many --- GetUserStatusCountsOverTime returns the count of users in each status over time. +-- name: GetUserStatusCounts :many +-- GetUserStatusCounts returns the count of users in each status over time. -- The time range is inclusively defined by the start_time and end_time parameters. -- -- Bucketing: diff --git a/coderd/insights.go b/coderd/insights.go index 50691c62514be..e9282050d345f 100644 --- a/coderd/insights.go +++ b/coderd/insights.go @@ -293,14 +293,14 @@ func (api *API) insightsUserLatency(rw http.ResponseWriter, r *http.Request) { } // @Summary Get insights about user status counts over time -// @ID get-insights-about-user-status-counts-over-time +// @ID get-insights-about-user-status-counts // @Security CoderSessionToken // @Produce json // @Tags Insights // @Param tz_offset query int true "Time-zone offset (e.g. -2)" -// @Success 200 {object} codersdk.GetUserStatusCountsOverTimeResponse -// @Router /insights/user-status-counts-over-time [get] -func (api *API) insightsUserStatusCountsOverTime(rw http.ResponseWriter, r *http.Request) { +// @Success 200 {object} codersdk.GetUserStatusCountsResponse +// @Router /insights/user-status-counts [get] +func (api *API) insightsUserStatusCounts(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() p := httpapi.NewQueryParamParser() @@ -324,7 +324,7 @@ func (api *API) insightsUserStatusCountsOverTime(rw http.ResponseWriter, r *http // Always return 60 days of data (2 months). sixtyDaysAgo := nextHourInLoc.In(loc).Truncate(24*time.Hour).AddDate(0, 0, -60) - rows, err := api.Database.GetUserStatusCountsOverTime(ctx, database.GetUserStatusCountsOverTimeParams{ + rows, err := api.Database.GetUserStatusCounts(ctx, database.GetUserStatusCountsParams{ StartTime: sixtyDaysAgo, EndTime: nextHourInLoc, }) @@ -340,7 +340,7 @@ func (api *API) insightsUserStatusCountsOverTime(rw http.ResponseWriter, r *http return } - resp := codersdk.GetUserStatusCountsOverTimeResponse{ + resp := codersdk.GetUserStatusCountsResponse{ StatusCounts: make(map[codersdk.UserStatus][]codersdk.UserStatusChangeCount), } diff --git a/codersdk/insights.go b/codersdk/insights.go index 3e285b5de0a58..ef44b6b8d013e 100644 --- a/codersdk/insights.go +++ b/codersdk/insights.go @@ -283,7 +283,7 @@ func (c *Client) TemplateInsights(ctx context.Context, req TemplateInsightsReque return result, json.NewDecoder(resp.Body).Decode(&result) } -type GetUserStatusCountsOverTimeResponse struct { +type GetUserStatusCountsResponse struct { StatusCounts map[UserStatus][]UserStatusChangeCount `json:"status_counts"` } @@ -292,24 +292,24 @@ type UserStatusChangeCount struct { Count int64 `json:"count" example:"10"` } -type GetUserStatusCountsOverTimeRequest struct { +type GetUserStatusCountsRequest struct { Offset time.Time `json:"offset" format:"date-time"` } -func (c *Client) GetUserStatusCountsOverTime(ctx context.Context, req GetUserStatusCountsOverTimeRequest) (GetUserStatusCountsOverTimeResponse, error) { +func (c *Client) GetUserStatusCounts(ctx context.Context, req GetUserStatusCountsRequest) (GetUserStatusCountsResponse, error) { qp := url.Values{} qp.Add("offset", req.Offset.Format(insightsTimeLayout)) - reqURL := fmt.Sprintf("/api/v2/insights/user-status-counts-over-time?%s", qp.Encode()) + reqURL := fmt.Sprintf("/api/v2/insights/user-status-counts?%s", qp.Encode()) resp, err := c.Request(ctx, http.MethodGet, reqURL, nil) if err != nil { - return GetUserStatusCountsOverTimeResponse{}, xerrors.Errorf("make request: %w", err) + return GetUserStatusCountsResponse{}, xerrors.Errorf("make request: %w", err) } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - return GetUserStatusCountsOverTimeResponse{}, ReadBodyAsError(resp) + return GetUserStatusCountsResponse{}, ReadBodyAsError(resp) } - var result GetUserStatusCountsOverTimeResponse + var result GetUserStatusCountsResponse return result, json.NewDecoder(resp.Body).Decode(&result) } diff --git a/docs/reference/api/insights.md b/docs/reference/api/insights.md index c092411c76bcd..70a8639a37592 100644 --- a/docs/reference/api/insights.md +++ b/docs/reference/api/insights.md @@ -267,12 +267,12 @@ To perform this operation, you must be authenticated. [Learn more](authenticatio ```shell # Example request using curl -curl -X GET http://coder-server:8080/api/v2/insights/user-status-counts-over-time?tz_offset=0 \ +curl -X GET http://coder-server:8080/api/v2/insights/user-status-counts?tz_offset=0 \ -H 'Accept: application/json' \ -H 'Coder-Session-Token: API_KEY' ``` -`GET /insights/user-status-counts-over-time` +`GET /insights/user-status-counts` ### Parameters @@ -305,8 +305,8 @@ curl -X GET http://coder-server:8080/api/v2/insights/user-status-counts-over-tim ### Responses -| Status | Meaning | Description | Schema | -|--------|---------------------------------------------------------|-------------|--------------------------------------------------------------------------------------------------------| -| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | [codersdk.GetUserStatusCountsOverTimeResponse](schemas.md#codersdkgetuserstatuscountsovertimeresponse) | +| Status | Meaning | Description | Schema | +|--------|---------------------------------------------------------|-------------|----------------------------------------------------------------------------------------| +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | [codersdk.GetUserStatusCountsResponse](schemas.md#codersdkgetuserstatuscountsresponse) | To perform this operation, you must be authenticated. [Learn more](authentication.md). diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index 0837a5348e977..542294a01fa37 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -3000,7 +3000,7 @@ Git clone makes use of this by parsing the URL from: 'Username for "https://gith |-------|--------|----------|--------------|-------------| | `key` | string | false | | | -## codersdk.GetUserStatusCountsOverTimeResponse +## codersdk.GetUserStatusCountsResponse ```json { diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 4956de8691ed7..acb5254a61a0a 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -882,6 +882,16 @@ export interface GenerateAPIKeyResponse { readonly key: string; } +// From codersdk/insights.go +export interface GetUserStatusCountsRequest { + readonly offset: string; +} + +// From codersdk/insights.go +export interface GetUserStatusCountsResponse { + readonly status_counts: Record; +} + // From codersdk/users.go export interface GetUsersResponse { readonly users: readonly User[]; @@ -2690,6 +2700,12 @@ export interface UserRoles { // From codersdk/users.go export type UserStatus = "active" | "dormant" | "suspended"; +// From codersdk/insights.go +export interface UserStatusChangeCount { + readonly date: string; + readonly count: number; +} + export const UserStatuses: UserStatus[] = ["active", "dormant", "suspended"]; // From codersdk/users.go From 9457ac8adff51c1e009cf0c748e1c83822297026 Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Thu, 9 Jan 2025 11:21:18 +0000 Subject: [PATCH 27/32] fix tests --- coderd/apidoc/docs.go | 2 +- coderd/apidoc/swagger.json | 2 +- coderd/insights.go | 2 +- docs/reference/api/insights.md | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 9fb17a59f43e5..15da8b7eb5c36 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -1411,7 +1411,7 @@ const docTemplate = `{ "tags": [ "Insights" ], - "summary": "Get insights about user status counts over time", + "summary": "Get insights about user status counts", "operationId": "get-insights-about-user-status-counts", "parameters": [ { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index ca2f869336a4d..df288ed1876c8 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -1228,7 +1228,7 @@ ], "produces": ["application/json"], "tags": ["Insights"], - "summary": "Get insights about user status counts over time", + "summary": "Get insights about user status counts", "operationId": "get-insights-about-user-status-counts", "parameters": [ { diff --git a/coderd/insights.go b/coderd/insights.go index e9282050d345f..e4695f50495fb 100644 --- a/coderd/insights.go +++ b/coderd/insights.go @@ -292,7 +292,7 @@ func (api *API) insightsUserLatency(rw http.ResponseWriter, r *http.Request) { httpapi.Write(ctx, rw, http.StatusOK, resp) } -// @Summary Get insights about user status counts over time +// @Summary Get insights about user status counts // @ID get-insights-about-user-status-counts // @Security CoderSessionToken // @Produce json diff --git a/docs/reference/api/insights.md b/docs/reference/api/insights.md index 70a8639a37592..b8fcdbbb1e776 100644 --- a/docs/reference/api/insights.md +++ b/docs/reference/api/insights.md @@ -261,7 +261,7 @@ curl -X GET http://coder-server:8080/api/v2/insights/user-latency?start_time=201 To perform this operation, you must be authenticated. [Learn more](authentication.md). -## Get insights about user status counts over time +## Get insights about user status counts ### Code samples From 63128a3660bfdd21d0b03e9435a9ff0a19bc9e9d Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Fri, 10 Jan 2025 12:42:58 +0200 Subject: [PATCH 28/32] Update coderd/database/queries/insights.sql Co-authored-by: Mathias Fredriksson --- coderd/database/queries/insights.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/database/queries/insights.sql b/coderd/database/queries/insights.sql index 02b454b9c34c9..925239525116e 100644 --- a/coderd/database/queries/insights.sql +++ b/coderd/database/queries/insights.sql @@ -890,5 +890,5 @@ SELECT ) AS count FROM ranked_status_change_per_user_per_date rscpupd CROSS JOIN statuses -GROUP BY date, statuses.new_status; +GROUP BY rscpupd.date, statuses.new_status; From 89f0a11c1b0398a399e52178dd60de0f362a4555 Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Mon, 13 Jan 2025 05:25:26 +0000 Subject: [PATCH 29/32] Provide basic durability against multiple deletions --- .../fixtures/000283_user_status_changes.up.sql | 2 -- coderd/database/queries.sql.go | 16 ++++++++++++---- coderd/database/queries/insights.sql | 16 ++++++++++++---- 3 files changed, 24 insertions(+), 10 deletions(-) diff --git a/coderd/database/migrations/testdata/fixtures/000283_user_status_changes.up.sql b/coderd/database/migrations/testdata/fixtures/000283_user_status_changes.up.sql index 3f7bcc21b16f0..9559fa3ad0df8 100644 --- a/coderd/database/migrations/testdata/fixtures/000283_user_status_changes.up.sql +++ b/coderd/database/migrations/testdata/fixtures/000283_user_status_changes.up.sql @@ -10,7 +10,6 @@ INSERT INTO rbac_roles, login_type, avatar_url, - deleted, last_seen_at, quiet_hours_schedule, theme_preference, @@ -30,7 +29,6 @@ INSERT INTO '{}', 'password', '', - false, '2024-01-01 00:00:00', '', '', diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 703dbb5fae717..0aba1390ef8bc 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3128,9 +3128,13 @@ latest_status_before_range AS ( usc.new_status, usc.changed_at FROM user_status_changes usc - LEFT JOIN user_deleted ud ON usc.user_id = ud.user_id + LEFT JOIN LATERAL ( + SELECT COUNT(*) > 0 AS deleted + FROM user_deleted ud + WHERE ud.user_id = usc.user_id AND ud.changed_at < usc.changed_at + ) AS ud ON usc.user_id = ud.user_id WHERE usc.changed_at < $1::timestamptz - AND (ud.user_id IS NULL OR ud.deleted_at > $1::timestamptz) + AND NOT ud.deleted ORDER BY usc.user_id, usc.changed_at DESC ), -- status_changes_during_range defines the status of each user during the start_time and end_time. @@ -3143,10 +3147,14 @@ status_changes_during_range AS ( usc.new_status, usc.changed_at FROM user_status_changes usc - LEFT JOIN user_deleted ud ON usc.user_id = ud.user_id + LEFT JOIN LATERAL ( + SELECT COUNT(*) > 0 AS deleted + FROM user_deleted ud + WHERE ud.user_id = usc.user_id AND ud.changed_at < usc.changed_at + ) AS ud ON usc.user_id = ud.user_id WHERE usc.changed_at >= $1::timestamptz AND usc.changed_at <= $2::timestamptz - AND (ud.user_id IS NULL OR usc.changed_at < ud.deleted_at) + AND NOT deleted ), -- relevant_status_changes defines the status of each user at any point in time. -- It includes the status of each user before the start_time, and the status of each user during the start_time and end_time. diff --git a/coderd/database/queries/insights.sql b/coderd/database/queries/insights.sql index 925239525116e..8e8ab9287d1c4 100644 --- a/coderd/database/queries/insights.sql +++ b/coderd/database/queries/insights.sql @@ -818,9 +818,13 @@ latest_status_before_range AS ( usc.new_status, usc.changed_at FROM user_status_changes usc - LEFT JOIN user_deleted ud ON usc.user_id = ud.user_id + LEFT JOIN LATERAL ( + SELECT COUNT(*) > 0 AS deleted + FROM user_deleted ud + WHERE ud.user_id = usc.user_id AND ud.changed_at < usc.changed_at + ) AS ud ON usc.user_id = ud.user_id WHERE usc.changed_at < @start_time::timestamptz - AND (ud.user_id IS NULL OR ud.deleted_at > @start_time::timestamptz) + AND NOT ud.deleted ORDER BY usc.user_id, usc.changed_at DESC ), -- status_changes_during_range defines the status of each user during the start_time and end_time. @@ -833,10 +837,14 @@ status_changes_during_range AS ( usc.new_status, usc.changed_at FROM user_status_changes usc - LEFT JOIN user_deleted ud ON usc.user_id = ud.user_id + LEFT JOIN LATERAL ( + SELECT COUNT(*) > 0 AS deleted + FROM user_deleted ud + WHERE ud.user_id = usc.user_id AND ud.changed_at < usc.changed_at + ) AS ud ON usc.user_id = ud.user_id WHERE usc.changed_at >= @start_time::timestamptz AND usc.changed_at <= @end_time::timestamptz - AND (ud.user_id IS NULL OR usc.changed_at < ud.deleted_at) + AND NOT deleted ), -- relevant_status_changes defines the status of each user at any point in time. -- It includes the status of each user before the start_time, and the status of each user during the start_time and end_time. From 89ebab23215fdbfddf43716aeb4a9c25dfa00c69 Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Mon, 13 Jan 2025 10:11:42 +0000 Subject: [PATCH 30/32] Provide basic durability against multiple deletions --- coderd/database/queries.sql.go | 20 +++++++++++--------- coderd/database/queries/insights.sql | 18 ++++++++++-------- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 0aba1390ef8bc..9301e4b6f725c 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3126,15 +3126,15 @@ latest_status_before_range AS ( SELECT DISTINCT usc.user_id, usc.new_status, - usc.changed_at + usc.changed_at, + ud.deleted FROM user_status_changes usc LEFT JOIN LATERAL ( SELECT COUNT(*) > 0 AS deleted FROM user_deleted ud - WHERE ud.user_id = usc.user_id AND ud.changed_at < usc.changed_at - ) AS ud ON usc.user_id = ud.user_id + WHERE ud.user_id = usc.user_id AND (ud.deleted_at < usc.changed_at OR ud.deleted_at < $1) + ) AS ud ON true WHERE usc.changed_at < $1::timestamptz - AND NOT ud.deleted ORDER BY usc.user_id, usc.changed_at DESC ), -- status_changes_during_range defines the status of each user during the start_time and end_time. @@ -3145,16 +3145,16 @@ status_changes_during_range AS ( SELECT usc.user_id, usc.new_status, - usc.changed_at + usc.changed_at, + ud.deleted FROM user_status_changes usc LEFT JOIN LATERAL ( SELECT COUNT(*) > 0 AS deleted FROM user_deleted ud - WHERE ud.user_id = usc.user_id AND ud.changed_at < usc.changed_at - ) AS ud ON usc.user_id = ud.user_id + WHERE ud.user_id = usc.user_id AND ud.deleted_at < usc.changed_at + ) AS ud ON true WHERE usc.changed_at >= $1::timestamptz AND usc.changed_at <= $2::timestamptz - AND NOT deleted ), -- relevant_status_changes defines the status of each user at any point in time. -- It includes the status of each user before the start_time, and the status of each user during the start_time and end_time. @@ -3164,6 +3164,7 @@ relevant_status_changes AS ( new_status, changed_at FROM latest_status_before_range + WHERE NOT deleted UNION ALL @@ -3172,6 +3173,7 @@ relevant_status_changes AS ( new_status, changed_at FROM status_changes_during_range + WHERE NOT deleted ), -- statuses defines all the distinct statuses that were present just before and during the time range. -- This is used to ensure that we have a series for every relevant status. @@ -3208,7 +3210,7 @@ SELECT ) AS count FROM ranked_status_change_per_user_per_date rscpupd CROSS JOIN statuses -GROUP BY date, statuses.new_status +GROUP BY rscpupd.date, statuses.new_status ` type GetUserStatusCountsParams struct { diff --git a/coderd/database/queries/insights.sql b/coderd/database/queries/insights.sql index 8e8ab9287d1c4..c6c6a78fc4b73 100644 --- a/coderd/database/queries/insights.sql +++ b/coderd/database/queries/insights.sql @@ -816,15 +816,15 @@ latest_status_before_range AS ( SELECT DISTINCT usc.user_id, usc.new_status, - usc.changed_at + usc.changed_at, + ud.deleted FROM user_status_changes usc LEFT JOIN LATERAL ( SELECT COUNT(*) > 0 AS deleted FROM user_deleted ud - WHERE ud.user_id = usc.user_id AND ud.changed_at < usc.changed_at - ) AS ud ON usc.user_id = ud.user_id + WHERE ud.user_id = usc.user_id AND (ud.deleted_at < usc.changed_at OR ud.deleted_at < @start_time) + ) AS ud ON true WHERE usc.changed_at < @start_time::timestamptz - AND NOT ud.deleted ORDER BY usc.user_id, usc.changed_at DESC ), -- status_changes_during_range defines the status of each user during the start_time and end_time. @@ -835,16 +835,16 @@ status_changes_during_range AS ( SELECT usc.user_id, usc.new_status, - usc.changed_at + usc.changed_at, + ud.deleted FROM user_status_changes usc LEFT JOIN LATERAL ( SELECT COUNT(*) > 0 AS deleted FROM user_deleted ud - WHERE ud.user_id = usc.user_id AND ud.changed_at < usc.changed_at - ) AS ud ON usc.user_id = ud.user_id + WHERE ud.user_id = usc.user_id AND ud.deleted_at < usc.changed_at + ) AS ud ON true WHERE usc.changed_at >= @start_time::timestamptz AND usc.changed_at <= @end_time::timestamptz - AND NOT deleted ), -- relevant_status_changes defines the status of each user at any point in time. -- It includes the status of each user before the start_time, and the status of each user during the start_time and end_time. @@ -854,6 +854,7 @@ relevant_status_changes AS ( new_status, changed_at FROM latest_status_before_range + WHERE NOT deleted UNION ALL @@ -862,6 +863,7 @@ relevant_status_changes AS ( new_status, changed_at FROM status_changes_during_range + WHERE NOT deleted ), -- statuses defines all the distinct statuses that were present just before and during the time range. -- This is used to ensure that we have a series for every relevant status. From 012f14c75f1034766f874b79f1960b6fcc6e3e41 Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Mon, 13 Jan 2025 10:34:25 +0000 Subject: [PATCH 31/32] populate the user_deleted_table --- coderd/database/migrations/000283_user_status_changes.up.sql | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/coderd/database/migrations/000283_user_status_changes.up.sql b/coderd/database/migrations/000283_user_status_changes.up.sql index 04d8472e55460..1d732a65076d1 100644 --- a/coderd/database/migrations/000283_user_status_changes.up.sql +++ b/coderd/database/migrations/000283_user_status_changes.up.sql @@ -31,6 +31,11 @@ COMMENT ON TABLE user_deleted IS 'Tracks when users were deleted'; CREATE INDEX idx_user_deleted_deleted_at ON user_deleted(deleted_at); +INSERT INTO user_deleted (user_id, deleted_at) +SELECT id, updated_at +FROM users +WHERE deleted; + CREATE OR REPLACE FUNCTION record_user_status_change() RETURNS trigger AS $$ BEGIN IF TG_OP = 'INSERT' OR OLD.status IS DISTINCT FROM NEW.status THEN From c2efd97ab24ee767692bf0d6f389d2b4d4e616ce Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Mon, 13 Jan 2025 10:40:55 +0000 Subject: [PATCH 32/32] formatting --- .../migrations/000283_user_status_changes.up.sql | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/coderd/database/migrations/000283_user_status_changes.up.sql b/coderd/database/migrations/000283_user_status_changes.up.sql index 1d732a65076d1..d712465851eff 100644 --- a/coderd/database/migrations/000283_user_status_changes.up.sql +++ b/coderd/database/migrations/000283_user_status_changes.up.sql @@ -31,8 +31,13 @@ COMMENT ON TABLE user_deleted IS 'Tracks when users were deleted'; CREATE INDEX idx_user_deleted_deleted_at ON user_deleted(deleted_at); -INSERT INTO user_deleted (user_id, deleted_at) -SELECT id, updated_at +INSERT INTO user_deleted ( + user_id, + deleted_at +) +SELECT + id, + updated_at FROM users WHERE deleted;