diff --git a/coderd/audit.go b/coderd/audit.go index d43f804e6a52d..54be5e3045b52 100644 --- a/coderd/audit.go +++ b/coderd/audit.go @@ -57,45 +57,19 @@ func (api *API) auditLogs(rw http.ResponseWriter, r *http.Request) { httpapi.InternalServerError(rw, err) return } - - httpapi.Write(ctx, rw, http.StatusOK, codersdk.AuditLogResponse{ - AuditLogs: convertAuditLogs(dblogs), - }) -} - -func (api *API) auditLogCount(rw http.ResponseWriter, r *http.Request) { - ctx := r.Context() - if !api.Authorize(r, rbac.ActionRead, rbac.ResourceAuditLog) { - httpapi.Forbidden(rw) - return - } - - queryStr := r.URL.Query().Get("q") - filter, errs := auditSearchQuery(queryStr) - if len(errs) > 0 { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Invalid audit search query.", - Validations: errs, + // GetAuditLogsOffset does not return ErrNoRows because it uses a window function to get the count. + // So we need to check if the dblogs is empty and return an empty array if so. + if len(dblogs) == 0 { + httpapi.Write(ctx, rw, http.StatusOK, codersdk.AuditLogResponse{ + AuditLogs: []codersdk.AuditLog{}, + Count: 0, }) return } - count, err := api.Database.GetAuditLogCount(ctx, database.GetAuditLogCountParams{ - ResourceType: filter.ResourceType, - ResourceID: filter.ResourceID, - Action: filter.Action, - Username: filter.Username, - Email: filter.Email, - DateFrom: filter.DateFrom, - DateTo: filter.DateTo, - }) - if err != nil { - httpapi.InternalServerError(rw, err) - return - } - - httpapi.Write(ctx, rw, http.StatusOK, codersdk.AuditLogCountResponse{ - Count: count, + httpapi.Write(ctx, rw, http.StatusOK, codersdk.AuditLogResponse{ + AuditLogs: convertAuditLogs(dblogs), + Count: dblogs[0].Count, }) } diff --git a/coderd/audit_test.go b/coderd/audit_test.go index 13c90bcceb08a..1fbcf7814f8e0 100644 --- a/coderd/audit_test.go +++ b/coderd/audit_test.go @@ -25,9 +25,6 @@ func TestAuditLogs(t *testing.T) { err := client.CreateTestAuditLog(ctx, codersdk.CreateTestAuditLogRequest{}) require.NoError(t, err) - count, err := client.AuditLogCount(ctx, codersdk.AuditLogCountRequest{}) - require.NoError(t, err) - alogs, err := client.AuditLogs(ctx, codersdk.AuditLogsRequest{ Pagination: codersdk.Pagination{ Limit: 1, @@ -35,7 +32,7 @@ func TestAuditLogs(t *testing.T) { }) require.NoError(t, err) - require.Equal(t, int64(1), count.Count) + require.Equal(t, int64(1), alogs.Count) require.Len(t, alogs.AuditLogs, 1) }) } @@ -161,16 +158,7 @@ func TestAuditLogsFilter(t *testing.T) { }) require.NoError(t, err, "fetch audit logs") require.Len(t, auditLogs.AuditLogs, testCase.ExpectedResult, "expected audit logs returned") - }) - - // Test count filtering - t.Run("GetCount"+testCase.Name, func(t *testing.T) { - t.Parallel() - response, err := client.AuditLogCount(ctx, codersdk.AuditLogCountRequest{ - SearchQuery: testCase.SearchQuery, - }) - require.NoError(t, err, "fetch audit logs count") - require.Equal(t, int(response.Count), testCase.ExpectedResult, "expected audit logs count returned") + require.Equal(t, testCase.ExpectedResult, int(auditLogs.Count), "expected audit log count returned") }) } }) diff --git a/coderd/coderd.go b/coderd/coderd.go index 3f7d3d7211321..1c8c3e00b399f 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -318,7 +318,6 @@ func New(options *Options) *API { ) r.Get("/", api.auditLogs) - r.Get("/count", api.auditLogCount) r.Post("/testgenerate", api.generateFakeAuditLog) }) r.Route("/files", func(r chi.Router) { diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index 953e8db9f8ee9..1064105ad3d97 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -3054,6 +3054,7 @@ func (q *fakeQuerier) GetAuditLogsOffset(ctx context.Context, arg database.GetAu UserCreatedAt: sql.NullTime{Time: user.CreatedAt, Valid: userValid}, UserStatus: user.Status, UserRoles: user.RBACRoles, + Count: 0, }) if len(logs) >= int(arg.Limit) { @@ -3061,52 +3062,12 @@ func (q *fakeQuerier) GetAuditLogsOffset(ctx context.Context, arg database.GetAu } } - return logs, nil -} - -func (q *fakeQuerier) GetAuditLogCount(_ context.Context, arg database.GetAuditLogCountParams) (int64, error) { - q.mutex.RLock() - defer q.mutex.RUnlock() - - logs := make([]database.AuditLog, 0) - - for _, alog := range q.auditLogs { - if arg.Action != "" && !strings.Contains(string(alog.Action), arg.Action) { - continue - } - if arg.ResourceType != "" && !strings.Contains(string(alog.ResourceType), arg.ResourceType) { - continue - } - if arg.ResourceID != uuid.Nil && alog.ResourceID != arg.ResourceID { - continue - } - if arg.Username != "" { - user, err := q.GetUserByID(context.Background(), alog.UserID) - if err == nil && !strings.EqualFold(arg.Username, user.Username) { - continue - } - } - if arg.Email != "" { - user, err := q.GetUserByID(context.Background(), alog.UserID) - if err == nil && !strings.EqualFold(arg.Email, user.Email) { - continue - } - } - if !arg.DateFrom.IsZero() { - if alog.Time.Before(arg.DateFrom) { - continue - } - } - if !arg.DateTo.IsZero() { - if alog.Time.After(arg.DateTo) { - continue - } - } - - logs = append(logs, alog) + count := int64(len(logs)) + for i := range logs { + logs[i].Count = count } - return int64(len(logs)), nil + return logs, nil } func (q *fakeQuerier) InsertAuditLog(_ context.Context, arg database.InsertAuditLogParams) (database.AuditLog, error) { diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 151cb0cde9f29..f208b000d3530 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -33,7 +33,6 @@ type sqlcQuerier interface { GetAPIKeysLastUsedAfter(ctx context.Context, lastUsed time.Time) ([]APIKey, error) GetActiveUserCount(ctx context.Context) (int64, error) GetAllOrganizationMembers(ctx context.Context, organizationID uuid.UUID) ([]User, error) - GetAuditLogCount(ctx context.Context, arg GetAuditLogCountParams) (int64, error) // GetAuditLogsBefore retrieves `row_limit` number of audit logs before the provided // ID. GetAuditLogsOffset(ctx context.Context, arg GetAuditLogsOffsetParams) ([]GetAuditLogsOffsetRow, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 60932097bb51b..0612f4eb22abe 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -364,89 +364,6 @@ func (q *sqlQuerier) UpdateAPIKeyByID(ctx context.Context, arg UpdateAPIKeyByIDP return err } -const getAuditLogCount = `-- name: GetAuditLogCount :one -SELECT - COUNT(*) as count -FROM - audit_logs -WHERE - -- Filter resource_type - CASE - WHEN $1 :: text != '' THEN - resource_type = $1 :: resource_type - ELSE true - END - -- Filter resource_id - AND CASE - WHEN $2 :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN - resource_id = $2 - ELSE true - END - -- Filter by resource_target - AND CASE - WHEN $3 :: text != '' THEN - resource_target = $3 - ELSE true - END - -- Filter action - AND CASE - WHEN $4 :: text != '' THEN - action = $4 :: audit_action - ELSE true - END - -- Filter by username - AND CASE - WHEN $5 :: text != '' THEN - user_id = (SELECT id from users WHERE users.username = $5 ) - ELSE true - END - -- Filter by user_email - AND CASE - WHEN $6 :: text != '' THEN - user_id = (SELECT id from users WHERE users.email = $6 ) - ELSE true - END - -- Filter by date_from - AND CASE - WHEN $7 :: timestamp with time zone != '0001-01-01 00:00:00' THEN - "time" >= $7 - ELSE true - END - -- Filter by date_to - AND CASE - WHEN $8 :: timestamp with time zone != '0001-01-01 00:00:00' THEN - "time" <= $8 - ELSE true - END -` - -type GetAuditLogCountParams struct { - ResourceType string `db:"resource_type" json:"resource_type"` - ResourceID uuid.UUID `db:"resource_id" json:"resource_id"` - ResourceTarget string `db:"resource_target" json:"resource_target"` - Action string `db:"action" json:"action"` - Username string `db:"username" json:"username"` - Email string `db:"email" json:"email"` - DateFrom time.Time `db:"date_from" json:"date_from"` - DateTo time.Time `db:"date_to" json:"date_to"` -} - -func (q *sqlQuerier) GetAuditLogCount(ctx context.Context, arg GetAuditLogCountParams) (int64, error) { - row := q.db.QueryRowContext(ctx, getAuditLogCount, - arg.ResourceType, - arg.ResourceID, - arg.ResourceTarget, - arg.Action, - arg.Username, - arg.Email, - arg.DateFrom, - arg.DateTo, - ) - var count int64 - err := row.Scan(&count) - return count, err -} - const getAuditLogsOffset = `-- name: GetAuditLogsOffset :many SELECT audit_logs.id, audit_logs.time, audit_logs.user_id, audit_logs.organization_id, audit_logs.ip, audit_logs.user_agent, audit_logs.resource_type, audit_logs.resource_id, audit_logs.resource_target, audit_logs.action, audit_logs.diff, audit_logs.status_code, audit_logs.additional_fields, audit_logs.request_id, audit_logs.resource_icon, @@ -455,7 +372,8 @@ SELECT users.created_at AS user_created_at, users.status AS user_status, users.rbac_roles AS user_roles, - users.avatar_url AS user_avatar_url + users.avatar_url AS user_avatar_url, + COUNT(audit_logs.*) OVER() AS count FROM audit_logs LEFT JOIN @@ -552,6 +470,7 @@ type GetAuditLogsOffsetRow struct { UserStatus UserStatus `db:"user_status" json:"user_status"` UserRoles []string `db:"user_roles" json:"user_roles"` UserAvatarUrl sql.NullString `db:"user_avatar_url" json:"user_avatar_url"` + Count int64 `db:"count" json:"count"` } // GetAuditLogsBefore retrieves `row_limit` number of audit logs before the provided @@ -598,6 +517,7 @@ func (q *sqlQuerier) GetAuditLogsOffset(ctx context.Context, arg GetAuditLogsOff &i.UserStatus, pq.Array(&i.UserRoles), &i.UserAvatarUrl, + &i.Count, ); err != nil { return nil, err } diff --git a/coderd/database/queries/auditlogs.sql b/coderd/database/queries/auditlogs.sql index e80a1d7ba71e0..920b93461d77d 100644 --- a/coderd/database/queries/auditlogs.sql +++ b/coderd/database/queries/auditlogs.sql @@ -8,7 +8,8 @@ SELECT users.created_at AS user_created_at, users.status AS user_status, users.rbac_roles AS user_roles, - users.avatar_url AS user_avatar_url + users.avatar_url AS user_avatar_url, + COUNT(audit_logs.*) OVER() AS count FROM audit_logs LEFT JOIN @@ -69,61 +70,6 @@ LIMIT OFFSET $2; --- name: GetAuditLogCount :one -SELECT - COUNT(*) as count -FROM - audit_logs -WHERE - -- Filter resource_type - CASE - WHEN @resource_type :: text != '' THEN - resource_type = @resource_type :: resource_type - ELSE true - END - -- Filter resource_id - AND CASE - WHEN @resource_id :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN - resource_id = @resource_id - ELSE true - END - -- Filter by resource_target - AND CASE - WHEN @resource_target :: text != '' THEN - resource_target = @resource_target - ELSE true - END - -- Filter action - AND CASE - WHEN @action :: text != '' THEN - action = @action :: audit_action - ELSE true - END - -- Filter by username - AND CASE - WHEN @username :: text != '' THEN - user_id = (SELECT id from users WHERE users.username = @username ) - ELSE true - END - -- Filter by user_email - AND CASE - WHEN @email :: text != '' THEN - user_id = (SELECT id from users WHERE users.email = @email ) - ELSE true - END - -- Filter by date_from - AND CASE - WHEN @date_from :: timestamp with time zone != '0001-01-01 00:00:00' THEN - "time" >= @date_from - ELSE true - END - -- Filter by date_to - AND CASE - WHEN @date_to :: timestamp with time zone != '0001-01-01 00:00:00' THEN - "time" <= @date_to - ELSE true - END; - -- name: InsertAuditLog :one INSERT INTO audit_logs ( diff --git a/codersdk/audit.go b/codersdk/audit.go index 8e77c875aeca5..f958411501599 100644 --- a/codersdk/audit.go +++ b/codersdk/audit.go @@ -113,14 +113,7 @@ type AuditLogsRequest struct { type AuditLogResponse struct { AuditLogs []AuditLog `json:"audit_logs"` -} - -type AuditLogCountRequest struct { - SearchQuery string `json:"q,omitempty"` -} - -type AuditLogCountResponse struct { - Count int64 `json:"count"` + Count int64 `json:"count"` } type CreateTestAuditLogRequest struct { @@ -159,35 +152,6 @@ func (c *Client) AuditLogs(ctx context.Context, req AuditLogsRequest) (AuditLogR return logRes, nil } -// AuditLogCount returns the count of all audit logs in the product. -func (c *Client) AuditLogCount(ctx context.Context, req AuditLogCountRequest) (AuditLogCountResponse, error) { - res, err := c.Request(ctx, http.MethodGet, "/api/v2/audit/count", nil, func(r *http.Request) { - q := r.URL.Query() - var params []string - if req.SearchQuery != "" { - params = append(params, req.SearchQuery) - } - q.Set("q", strings.Join(params, " ")) - r.URL.RawQuery = q.Encode() - }) - if err != nil { - return AuditLogCountResponse{}, err - } - defer res.Body.Close() - - if res.StatusCode != http.StatusOK { - return AuditLogCountResponse{}, readBodyAsError(res) - } - - var logRes AuditLogCountResponse - err = json.NewDecoder(res.Body).Decode(&logRes) - if err != nil { - return AuditLogCountResponse{}, err - } - - return logRes, nil -} - func (c *Client) CreateTestAuditLog(ctx context.Context, req CreateTestAuditLogRequest) error { res, err := c.Request(ctx, http.MethodPost, "/api/v2/audit/testgenerate", req) if err != nil { diff --git a/site/src/api/api.ts b/site/src/api/api.ts index 8b344af736936..1ee53a929d1ae 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -556,14 +556,6 @@ export const getAuditLogs = async ( return response.data } -export const getAuditLogsCount = async ( - options: TypesGen.AuditLogCountRequest = {}, -): Promise => { - const url = getURLWithSearchParams("/api/v2/audit/count", options) - const response = await axios.get(url) - return response.data -} - export const getTemplateDAUs = async ( templateId: string, ): Promise => { diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index e745225e1c318..eb750ac5920e9 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -70,19 +70,10 @@ export interface AuditLog { readonly user?: User } -// From codersdk/audit.go -export interface AuditLogCountRequest { - readonly q?: string -} - -// From codersdk/audit.go -export interface AuditLogCountResponse { - readonly count: number -} - // From codersdk/audit.go export interface AuditLogResponse { readonly audit_logs: AuditLog[] + readonly count: number } // From codersdk/audit.go diff --git a/site/src/components/AuditLogRow/AuditLogRow.test.tsx b/site/src/components/AuditLogRow/AuditLogRow.test.tsx index 3ee13f46e4bc0..ab0152730161a 100644 --- a/site/src/components/AuditLogRow/AuditLogRow.test.tsx +++ b/site/src/components/AuditLogRow/AuditLogRow.test.tsx @@ -35,7 +35,7 @@ describe("readableActionMessage()", () => { // Then expect(friendlyString).toBe( - "TestUser updated workspace bruno-dev", + "TestUser created workspace bruno-dev", ) }) }) diff --git a/site/src/pages/AuditPage/AuditPage.test.tsx b/site/src/pages/AuditPage/AuditPage.test.tsx index 3234c7c67e3dc..490f0a801fef9 100644 --- a/site/src/pages/AuditPage/AuditPage.test.tsx +++ b/site/src/pages/AuditPage/AuditPage.test.tsx @@ -29,33 +29,22 @@ describe("AuditPage", () => { describe("Filtering", () => { it("filters by typing", async () => { - const getAuditLogsSpy = jest - .spyOn(API, "getAuditLogs") - .mockResolvedValue({ audit_logs: [MockAuditLog] }) - render() await waitForLoaderToBeRemoved() - - // Reset spy so we can focus on the call with the filter - getAuditLogsSpy.mockReset() + await screen.findByText("updated", { exact: false }) const filterField = screen.getByLabelText("Filter") const query = "resource_type:workspace action:create" await userEvent.type(filterField, query) - - await waitFor(() => - expect(getAuditLogsSpy).toBeCalledWith({ - limit: 25, - offset: 0, - q: query, - }), - ) + await screen.findByText("created", { exact: false }) + const editWorkspace = screen.queryByText("updated", { exact: false }) + expect(editWorkspace).not.toBeInTheDocument() }) it("filters by URL", async () => { const getAuditLogsSpy = jest .spyOn(API, "getAuditLogs") - .mockResolvedValue({ audit_logs: [MockAuditLog] }) + .mockResolvedValue({ audit_logs: [MockAuditLog], count: 1 }) const query = "resource_type:workspace action:create" history.push(`/audit?filter=${encodeURIComponent(query)}`) @@ -67,15 +56,11 @@ describe("AuditPage", () => { }) it("resets page to 1 when filter is changed", async () => { - const getAuditLogsSpy = jest - .spyOn(API, "getAuditLogs") - .mockResolvedValue({ audit_logs: [MockAuditLog] }) - history.push(`/audit?page=2`) render() await waitForLoaderToBeRemoved() - getAuditLogsSpy.mockReset() + const getAuditLogsSpy = jest.spyOn(API, "getAuditLogs") const filterField = screen.getByLabelText("Filter") const query = "resource_type:workspace action:create" diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index e56b5358a0aaa..d23918e05aaa2 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -956,7 +956,7 @@ export const MockAuditLog: TypesGen.AuditLog = { }, status_code: 200, additional_fields: {}, - description: "{user} updated workspace {target}", + description: "{user} created workspace {target}", user: MockUser, } @@ -964,6 +964,8 @@ export const MockAuditLog2: TypesGen.AuditLog = { ...MockAuditLog, id: "53bded77-7b9d-4e82-8771-991a34d759f9", action: "write", + time: "2022-05-20T16:45:57.122Z", + description: "{user} updated workspace {target}", diff: { workspace_name: { old: "old-workspace-name", diff --git a/site/src/testHelpers/handlers.ts b/site/src/testHelpers/handlers.ts index 173dfa58945d7..30e45fa727a2b 100644 --- a/site/src/testHelpers/handlers.ts +++ b/site/src/testHelpers/handlers.ts @@ -212,18 +212,20 @@ export const handlers = [ // Audit rest.get("/api/v2/audit", (req, res, ctx) => { + const filter = req.url.searchParams.get("q") as string + const logs = + filter === "resource_type:workspace action:create" + ? [M.MockAuditLog] + : [M.MockAuditLog, M.MockAuditLog2] return res( ctx.status(200), ctx.json({ - audit_logs: [M.MockAuditLog, M.MockAuditLog2], + audit_logs: logs, + count: logs.length, }), ) }), - rest.get("/api/v2/audit/count", (req, res, ctx) => { - return res(ctx.status(200), ctx.json({ count: 1000 })) - }), - // Applications host rest.get("/api/v2/applications/host", (req, res, ctx) => { return res(ctx.status(200), ctx.json({ host: "*.dev.coder.com" })) diff --git a/site/src/xServices/audit/auditXService.ts b/site/src/xServices/audit/auditXService.ts index dd86f32375d96..0aa245489a2b1 100644 --- a/site/src/xServices/audit/auditXService.ts +++ b/site/src/xServices/audit/auditXService.ts @@ -1,6 +1,6 @@ -import { getAuditLogs, getAuditLogsCount } from "api/api" +import { getAuditLogs } from "api/api" import { getErrorMessage } from "api/errors" -import { AuditLog } from "api/typesGenerated" +import { AuditLog, AuditLogResponse } from "api/typesGenerated" import { displayError } from "components/GlobalSnackbar/utils" import { getPaginationData } from "components/PaginationWidget/utils" import { @@ -29,10 +29,7 @@ export const auditMachine = createMachine( context: {} as AuditContext, services: {} as { loadAuditLogsAndCount: { - data: { - auditLogs: AuditLog[] - count: number - } + data: AuditLogResponse } }, events: {} as @@ -59,17 +56,17 @@ export const auditMachine = createMachine( invoke: { src: "loadAuditLogsAndCount", onDone: { - target: "success", + target: "idle", actions: ["assignAuditLogsAndCount"], }, onError: { - target: "error", + target: "idle", actions: ["displayApiError"], }, }, - onDone: "success", + onDone: "idle", }, - success: { + idle: { on: { UPDATE_PAGE: { actions: ["updateURL"], @@ -80,9 +77,6 @@ export const auditMachine = createMachine( }, }, }, - error: { - type: "final", - }, }, }, { @@ -91,7 +85,7 @@ export const auditMachine = createMachine( auditLogs: (_) => undefined, }), assignAuditLogsAndCount: assign({ - auditLogs: (_, event) => event.data.auditLogs, + auditLogs: (_, event) => event.data.audit_logs, count: (_, event) => event.data.count, }), assignPaginationRef: assign({ @@ -117,21 +111,11 @@ export const auditMachine = createMachine( loadAuditLogsAndCount: async (context) => { if (context.paginationRef) { const { offset, limit } = getPaginationData(context.paginationRef) - const [auditLogs, count] = await Promise.all([ - getAuditLogs({ - offset, - limit, - q: context.filter, - }).then((data) => data.audit_logs), - getAuditLogsCount({ - q: context.filter, - }).then((data) => data.count), - ]) - - return { - auditLogs, - count, - } + return getAuditLogs({ + offset, + limit, + q: context.filter, + }) } else { throw new Error("Cannot get audit logs without pagination data") }