Skip to content

Commit 06ed8c7

Browse files
ssncferreirassncferreira
committed
chore: address comments
1 parent b20b801 commit 06ed8c7

9 files changed

Lines changed: 201 additions & 39 deletions

File tree

‎coderd/aitasks_test.go‎

Lines changed: 56 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"github.com/coder/coder/v2/coderd/database"
2020
"github.com/coder/coder/v2/coderd/database/dbauthz"
2121
"github.com/coder/coder/v2/coderd/database/dbfake"
22+
"github.com/coder/coder/v2/coderd/database/dbgen"
2223
"github.com/coder/coder/v2/coderd/database/dbtestutil"
2324
"github.com/coder/coder/v2/coderd/notifications"
2425
"github.com/coder/coder/v2/coderd/notifications/notificationstest"
@@ -932,37 +933,62 @@ func TestTasksNotification(t *testing.T) {
932933

933934
for _, tc := range []struct {
934935
name string
935-
appStatus codersdk.WorkspaceAppStatusState
936+
latestAppStatuses []codersdk.WorkspaceAppStatusState
937+
newAppStatus codersdk.WorkspaceAppStatusState
936938
isAITask bool
937939
isNotificationSent bool
938940
notificationTemplate uuid.UUID
939941
}{
940942
// Should not send a notification when the agent app is not an AI task.
941943
{
942944
name: "NoAITask",
943-
appStatus: codersdk.WorkspaceAppStatusStateWorking,
945+
latestAppStatuses: nil,
946+
newAppStatus: codersdk.WorkspaceAppStatusStateWorking,
944947
isAITask: false,
945948
isNotificationSent: false,
946949
},
947-
// Should not send a notification when the app status is neither 'Working' nor 'Idle'.
950+
// Should not send a notification when the new app status is neither 'Working' nor 'Idle'.
948951
{
949-
name: "NoNotifiedStatus",
950-
appStatus: codersdk.WorkspaceAppStatusStateComplete,
952+
name: "NonNotifiedState",
953+
latestAppStatuses: nil,
954+
newAppStatus: codersdk.WorkspaceAppStatusStateComplete,
955+
isAITask: true,
956+
isNotificationSent: false,
957+
},
958+
// Should not send a notification when the new app status equals the latest status (Working).
959+
{
960+
name: "NonNotifiedTransition",
961+
latestAppStatuses: []codersdk.WorkspaceAppStatusState{codersdk.WorkspaceAppStatusStateWorking},
962+
newAppStatus: codersdk.WorkspaceAppStatusStateWorking,
951963
isAITask: true,
952964
isNotificationSent: false,
953965
},
954966
// Should send TemplateTaskWorking when the AI task transitions to 'Working'.
955967
{
956968
name: "TemplateTaskWorking",
957-
appStatus: codersdk.WorkspaceAppStatusStateWorking,
969+
latestAppStatuses: nil,
970+
newAppStatus: codersdk.WorkspaceAppStatusStateWorking,
971+
isAITask: true,
972+
isNotificationSent: true,
973+
notificationTemplate: notifications.TemplateTaskWorking,
974+
},
975+
// Should send TemplateTaskWorking when the AI task transitions to 'Working' from 'Idle'.
976+
{
977+
name: "TemplateTaskWorkingFromIdle",
978+
latestAppStatuses: []codersdk.WorkspaceAppStatusState{
979+
codersdk.WorkspaceAppStatusStateWorking,
980+
codersdk.WorkspaceAppStatusStateIdle,
981+
}, // latest
982+
newAppStatus: codersdk.WorkspaceAppStatusStateWorking,
958983
isAITask: true,
959984
isNotificationSent: true,
960985
notificationTemplate: notifications.TemplateTaskWorking,
961986
},
962987
// Should send TemplateTaskIdle when the AI task transitions to 'Idle'.
963988
{
964989
name: "TemplateTaskIdle",
965-
appStatus: codersdk.WorkspaceAppStatusStateIdle,
990+
latestAppStatuses: []codersdk.WorkspaceAppStatusState{codersdk.WorkspaceAppStatusStateWorking},
991+
newAppStatus: codersdk.WorkspaceAppStatusStateIdle,
966992
isAITask: true,
967993
isNotificationSent: true,
968994
notificationTemplate: notifications.TemplateTaskIdle,
@@ -978,12 +1004,12 @@ func TestTasksNotification(t *testing.T) {
9781004
NotificationsEnqueuer: notifyEnq,
9791005
})
9801006

981-
// Given: A member user
1007+
// Given: a member user
9821008
ownerUser := coderdtest.CreateFirstUser(t, client)
9831009
client, memberUser := coderdtest.CreateAnotherUser(t, client, ownerUser.OrganizationID)
9841010

9851011
// Given: a workspace build with an agent containing an App
986-
workspaceAgentID := uuid.NewString()
1012+
workspaceAgentAppID := uuid.New()
9871013
workspaceBuildID := uuid.New()
9881014
workspaceBuildSeed := database.WorkspaceBuild{
9891015
ID: workspaceBuildID,
@@ -993,7 +1019,7 @@ func TestTasksNotification(t *testing.T) {
9931019
ID: workspaceBuildID,
9941020
// AI Task configuration
9951021
HasAITask: sql.NullBool{Bool: true, Valid: true},
996-
AITaskSidebarAppID: uuid.NullUUID{UUID: uuid.MustParse(workspaceAgentID), Valid: true},
1022+
AITaskSidebarAppID: uuid.NullUUID{UUID: workspaceAgentAppID, Valid: true},
9971023
}
9981024
}
9991025
workspaceBuild := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{
@@ -1005,19 +1031,32 @@ func TestTasksNotification(t *testing.T) {
10051031
Value: "task prompt",
10061032
}).WithAgent(func(agent []*proto.Agent) []*proto.Agent {
10071033
agent[0].Apps = []*proto.App{{
1008-
Id: workspaceAgentID,
1034+
Id: workspaceAgentAppID.String(),
10091035
Slug: "ccw",
10101036
}}
10111037
return agent
10121038
}).Do()
10131039

1014-
// When: The agent updates the app status
1040+
// Given: the workspace agent app has previous statuses
10151041
agentClient := agentsdk.New(client.URL, agentsdk.WithFixedToken(workspaceBuild.AgentToken))
1042+
if len(tc.latestAppStatuses) > 0 {
1043+
workspace := coderdtest.MustWorkspace(t, client, workspaceBuild.Workspace.ID)
1044+
for _, appStatus := range tc.latestAppStatuses {
1045+
dbgen.WorkspaceAppStatus(t, db, database.WorkspaceAppStatus{
1046+
WorkspaceID: workspaceBuild.Workspace.ID,
1047+
AgentID: workspace.LatestBuild.Resources[0].Agents[0].ID,
1048+
AppID: workspaceAgentAppID,
1049+
State: database.WorkspaceAppStatusState(appStatus),
1050+
})
1051+
}
1052+
}
1053+
1054+
// When: the agent updates the app status
10161055
err := agentClient.PatchAppStatus(ctx, agentsdk.PatchAppStatus{
10171056
AppSlug: "ccw",
10181057
Message: "testing",
10191058
URI: "https://example.com",
1020-
State: tc.appStatus,
1059+
State: tc.newAppStatus,
10211060
})
10221061
require.NoError(t, err)
10231062

@@ -1026,8 +1065,10 @@ func TestTasksNotification(t *testing.T) {
10261065
require.NoError(t, err)
10271066
workspaceAgent, err := client.WorkspaceAgent(ctx, workspace.LatestBuild.Resources[0].Agents[0].ID)
10281067
require.NoError(t, err)
1029-
require.Len(t, workspaceAgent.Apps[0].Statuses, 1)
1030-
require.Equal(t, workspaceAgent.Apps[0].Statuses[0].State, tc.appStatus)
1068+
require.Len(t, workspaceAgent.Apps, 1)
1069+
require.GreaterOrEqual(t, len(workspaceAgent.Apps[0].Statuses), 1)
1070+
latestStatusIndex := len(workspaceAgent.Apps[0].Statuses) - 1
1071+
require.Equal(t, tc.newAppStatus, workspaceAgent.Apps[0].Statuses[latestStatusIndex].State)
10311072

10321073
if tc.isNotificationSent {
10331074
// Then: A notification is sent to the workspace owner (memberUser)

‎coderd/database/dbauthz/dbauthz.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2313,6 +2313,13 @@ func (q *querier) GetLatestCryptoKeyByFeature(ctx context.Context, feature datab
23132313
return q.db.GetLatestCryptoKeyByFeature(ctx, feature)
23142314
}
23152315

2316+
func (q *querier) GetLatestWorkspaceAppStatusesByAppID(ctx context.Context, appID uuid.UUID) ([]database.WorkspaceAppStatus, error) {
2317+
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil {
2318+
return nil, err
2319+
}
2320+
return q.db.GetLatestWorkspaceAppStatusesByAppID(ctx, appID)
2321+
}
2322+
23162323
func (q *querier) GetLatestWorkspaceAppStatusesByWorkspaceIDs(ctx context.Context, ids []uuid.UUID) ([]database.WorkspaceAppStatus, error) {
23172324
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil {
23182325
return nil, err

‎coderd/database/dbgen/dbgen.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -905,6 +905,21 @@ func WorkspaceAppStat(t testing.TB, db database.Store, orig database.WorkspaceAp
905905
return scheme
906906
}
907907

908+
func WorkspaceAppStatus(t testing.TB, db database.Store, orig database.WorkspaceAppStatus) database.WorkspaceAppStatus {
909+
appStatus, err := db.InsertWorkspaceAppStatus(genCtx, database.InsertWorkspaceAppStatusParams{
910+
ID: takeFirst(orig.ID, uuid.New()),
911+
CreatedAt: takeFirst(orig.CreatedAt, dbtime.Now()),
912+
WorkspaceID: takeFirst(orig.WorkspaceID, uuid.New()),
913+
AgentID: takeFirst(orig.AgentID, uuid.New()),
914+
AppID: takeFirst(orig.AppID, uuid.New()),
915+
State: takeFirst(orig.State, database.WorkspaceAppStatusStateWorking),
916+
Message: takeFirst(orig.Message, ""),
917+
Uri: takeFirst(orig.Uri, sql.NullString{}),
918+
})
919+
require.NoError(t, err, "insert workspace agent status")
920+
return appStatus
921+
}
922+
908923
func WorkspaceResource(t testing.TB, db database.Store, orig database.WorkspaceResource) database.WorkspaceResource {
909924
resource, err := db.InsertWorkspaceResource(genCtx, database.InsertWorkspaceResourceParams{
910925
ID: takeFirst(orig.ID, uuid.New()),

‎coderd/database/dbmetrics/querymetrics.go‎

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/dbmock/dbmock.go‎

Lines changed: 15 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/querier.go‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries.sql.go‎

Lines changed: 39 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/workspaceapps.sql‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,12 @@ RETURNING *;
7373
-- name: GetWorkspaceAppStatusesByAppIDs :many
7474
SELECT * FROM workspace_app_statuses WHERE app_id = ANY(@ids :: uuid [ ]);
7575

76+
-- name: GetLatestWorkspaceAppStatusesByAppID :many
77+
SELECT *
78+
FROM workspace_app_statuses
79+
WHERE app_id = @app_id::uuid
80+
ORDER BY created_at DESC, id DESC;
81+
7682
-- name: GetLatestWorkspaceAppStatusesByWorkspaceIDs :many
7783
SELECT DISTINCT ON (workspace_id)
7884
*

0 commit comments

Comments
 (0)