Skip to content

Commit 6b109d0

Browse files
presleyppull[bot]
presleyp
authored and
pull[bot]
committed
chore: refactor audit page to use window function for count (#5133)
* Move count query to window function * Unpack count and update types * Remove count endpoint * Update tests, wip * Fix tests * Update frontend, wip * Remove space * Fix frontend test * Don't hang on error * Handle no results * Don't omit count * Fix frontend tests
1 parent 5e98077 commit 6b109d0

File tree

15 files changed

+54
-347
lines changed

15 files changed

+54
-347
lines changed

‎coderd/audit.go

Lines changed: 9 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -57,45 +57,19 @@ func (api *API) auditLogs(rw http.ResponseWriter, r *http.Request) {
5757
httpapi.InternalServerError(rw, err)
5858
return
5959
}
60-
61-
httpapi.Write(ctx, rw, http.StatusOK, codersdk.AuditLogResponse{
62-
AuditLogs: convertAuditLogs(dblogs),
63-
})
64-
}
65-
66-
func (api *API) auditLogCount(rw http.ResponseWriter, r *http.Request) {
67-
ctx := r.Context()
68-
if !api.Authorize(r, rbac.ActionRead, rbac.ResourceAuditLog) {
69-
httpapi.Forbidden(rw)
70-
return
71-
}
72-
73-
queryStr := r.URL.Query().Get("q")
74-
filter, errs := auditSearchQuery(queryStr)
75-
if len(errs) > 0 {
76-
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
77-
Message: "Invalid audit search query.",
78-
Validations: errs,
60+
// GetAuditLogsOffset does not return ErrNoRows because it uses a window function to get the count.
61+
// So we need to check if the dblogs is empty and return an empty array if so.
62+
if len(dblogs) == 0 {
63+
httpapi.Write(ctx, rw, http.StatusOK, codersdk.AuditLogResponse{
64+
AuditLogs: []codersdk.AuditLog{},
65+
Count: 0,
7966
})
8067
return
8168
}
8269

83-
count, err := api.Database.GetAuditLogCount(ctx, database.GetAuditLogCountParams{
84-
ResourceType: filter.ResourceType,
85-
ResourceID: filter.ResourceID,
86-
Action: filter.Action,
87-
Username: filter.Username,
88-
Email: filter.Email,
89-
DateFrom: filter.DateFrom,
90-
DateTo: filter.DateTo,
91-
})
92-
if err != nil {
93-
httpapi.InternalServerError(rw, err)
94-
return
95-
}
96-
97-
httpapi.Write(ctx, rw, http.StatusOK, codersdk.AuditLogCountResponse{
98-
Count: count,
70+
httpapi.Write(ctx, rw, http.StatusOK, codersdk.AuditLogResponse{
71+
AuditLogs: convertAuditLogs(dblogs),
72+
Count: dblogs[0].Count,
9973
})
10074
}
10175

‎coderd/audit_test.go

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -25,17 +25,14 @@ func TestAuditLogs(t *testing.T) {
2525
err := client.CreateTestAuditLog(ctx, codersdk.CreateTestAuditLogRequest{})
2626
require.NoError(t, err)
2727

28-
count, err := client.AuditLogCount(ctx, codersdk.AuditLogCountRequest{})
29-
require.NoError(t, err)
30-
3128
alogs, err := client.AuditLogs(ctx, codersdk.AuditLogsRequest{
3229
Pagination: codersdk.Pagination{
3330
Limit: 1,
3431
},
3532
})
3633
require.NoError(t, err)
3734

38-
require.Equal(t, int64(1), count.Count)
35+
require.Equal(t, int64(1), alogs.Count)
3936
require.Len(t, alogs.AuditLogs, 1)
4037
})
4138
}
@@ -161,16 +158,7 @@ func TestAuditLogsFilter(t *testing.T) {
161158
})
162159
require.NoError(t, err, "fetch audit logs")
163160
require.Len(t, auditLogs.AuditLogs, testCase.ExpectedResult, "expected audit logs returned")
164-
})
165-
166-
// Test count filtering
167-
t.Run("GetCount"+testCase.Name, func(t *testing.T) {
168-
t.Parallel()
169-
response, err := client.AuditLogCount(ctx, codersdk.AuditLogCountRequest{
170-
SearchQuery: testCase.SearchQuery,
171-
})
172-
require.NoError(t, err, "fetch audit logs count")
173-
require.Equal(t, int(response.Count), testCase.ExpectedResult, "expected audit logs count returned")
161+
require.Equal(t, testCase.ExpectedResult, int(auditLogs.Count), "expected audit log count returned")
174162
})
175163
}
176164
})

‎coderd/coderd.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,6 @@ func New(options *Options) *API {
318318
)
319319

320320
r.Get("/", api.auditLogs)
321-
r.Get("/count", api.auditLogCount)
322321
r.Post("/testgenerate", api.generateFakeAuditLog)
323322
})
324323
r.Route("/files", func(r chi.Router) {

‎coderd/database/databasefake/databasefake.go

Lines changed: 5 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -3100,59 +3100,20 @@ func (q *fakeQuerier) GetAuditLogsOffset(ctx context.Context, arg database.GetAu
31003100
UserCreatedAt: sql.NullTime{Time: user.CreatedAt, Valid: userValid},
31013101
UserStatus: user.Status,
31023102
UserRoles: user.RBACRoles,
3103+
Count: 0,
31033104
})
31043105

31053106
if len(logs) >= int(arg.Limit) {
31063107
break
31073108
}
31083109
}
31093110

3110-
return logs, nil
3111-
}
3112-
3113-
func (q *fakeQuerier) GetAuditLogCount(_ context.Context, arg database.GetAuditLogCountParams) (int64, error) {
3114-
q.mutex.RLock()
3115-
defer q.mutex.RUnlock()
3116-
3117-
logs := make([]database.AuditLog, 0)
3118-
3119-
for _, alog := range q.auditLogs {
3120-
if arg.Action != "" && !strings.Contains(string(alog.Action), arg.Action) {
3121-
continue
3122-
}
3123-
if arg.ResourceType != "" && !strings.Contains(string(alog.ResourceType), arg.ResourceType) {
3124-
continue
3125-
}
3126-
if arg.ResourceID != uuid.Nil && alog.ResourceID != arg.ResourceID {
3127-
continue
3128-
}
3129-
if arg.Username != "" {
3130-
user, err := q.GetUserByID(context.Background(), alog.UserID)
3131-
if err == nil && !strings.EqualFold(arg.Username, user.Username) {
3132-
continue
3133-
}
3134-
}
3135-
if arg.Email != "" {
3136-
user, err := q.GetUserByID(context.Background(), alog.UserID)
3137-
if err == nil && !strings.EqualFold(arg.Email, user.Email) {
3138-
continue
3139-
}
3140-
}
3141-
if !arg.DateFrom.IsZero() {
3142-
if alog.Time.Before(arg.DateFrom) {
3143-
continue
3144-
}
3145-
}
3146-
if !arg.DateTo.IsZero() {
3147-
if alog.Time.After(arg.DateTo) {
3148-
continue
3149-
}
3150-
}
3151-
3152-
logs = append(logs, alog)
3111+
count := int64(len(logs))
3112+
for i := range logs {
3113+
logs[i].Count = count
31533114
}
31543115

3155-
return int64(len(logs)), nil
3116+
return logs, nil
31563117
}
31573118

31583119
func (q *fakeQuerier) InsertAuditLog(_ context.Context, arg database.InsertAuditLogParams) (database.AuditLog, error) {

‎coderd/database/querier.go

Lines changed: 0 additions & 1 deletion
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: 4 additions & 84 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/auditlogs.sql

Lines changed: 2 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ SELECT
88
users.created_at AS user_created_at,
99
users.status AS user_status,
1010
users.rbac_roles AS user_roles,
11-
users.avatar_url AS user_avatar_url
11+
users.avatar_url AS user_avatar_url,
12+
COUNT(audit_logs.*) OVER() AS count
1213
FROM
1314
audit_logs
1415
LEFT JOIN
@@ -69,61 +70,6 @@ LIMIT
6970
OFFSET
7071
$2;
7172

72-
-- name: GetAuditLogCount :one
73-
SELECT
74-
COUNT(*) as count
75-
FROM
76-
audit_logs
77-
WHERE
78-
-- Filter resource_type
79-
CASE
80-
WHEN @resource_type :: text != '' THEN
81-
resource_type = @resource_type :: resource_type
82-
ELSE true
83-
END
84-
-- Filter resource_id
85-
AND CASE
86-
WHEN @resource_id :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN
87-
resource_id = @resource_id
88-
ELSE true
89-
END
90-
-- Filter by resource_target
91-
AND CASE
92-
WHEN @resource_target :: text != '' THEN
93-
resource_target = @resource_target
94-
ELSE true
95-
END
96-
-- Filter action
97-
AND CASE
98-
WHEN @action :: text != '' THEN
99-
action = @action :: audit_action
100-
ELSE true
101-
END
102-
-- Filter by username
103-
AND CASE
104-
WHEN @username :: text != '' THEN
105-
user_id = (SELECT id from users WHERE users.username = @username )
106-
ELSE true
107-
END
108-
-- Filter by user_email
109-
AND CASE
110-
WHEN @email :: text != '' THEN
111-
user_id = (SELECT id from users WHERE users.email = @email )
112-
ELSE true
113-
END
114-
-- Filter by date_from
115-
AND CASE
116-
WHEN @date_from :: timestamp with time zone != '0001-01-01 00:00:00' THEN
117-
"time" >= @date_from
118-
ELSE true
119-
END
120-
-- Filter by date_to
121-
AND CASE
122-
WHEN @date_to :: timestamp with time zone != '0001-01-01 00:00:00' THEN
123-
"time" <= @date_to
124-
ELSE true
125-
END;
126-
12773
-- name: InsertAuditLog :one
12874
INSERT INTO
12975
audit_logs (

0 commit comments

Comments
 (0)