From 20c6bfa39437154d78a39e73228a225dcdc15bb7 Mon Sep 17 00:00:00 2001 From: Kira Pilot Date: Wed, 12 Oct 2022 14:17:10 +0000 Subject: [PATCH 1/5] fix: duplicate workspace update entries --- coderd/audit/request.go | 4 +++ site/src/pages/AuditPage/AuditPage.test.tsx | 18 ++++++++++++ .../WorkspaceSchedulePage.tsx | 3 +- site/src/testHelpers/entities.ts | 28 ++++++++++++++++++- site/src/xServices/audit/auditXService.ts | 4 ++- .../workspaceScheduleXService.ts | 9 +++--- 6 files changed, 58 insertions(+), 8 deletions(-) diff --git a/coderd/audit/request.go b/coderd/audit/request.go index c23ad2b1f7339..a37ddd7e22224 100644 --- a/coderd/audit/request.go +++ b/coderd/audit/request.go @@ -115,6 +115,10 @@ func InitRequest[T Auditable](w http.ResponseWriter, p *RequestParams) (*Request if sw.Status < 400 { diff := Diff(p.Audit, req.Old, req.New) + if len(diff) == 0 { + return + } + var err error diffRaw, err = json.Marshal(diff) if err != nil { diff --git a/site/src/pages/AuditPage/AuditPage.test.tsx b/site/src/pages/AuditPage/AuditPage.test.tsx index 7e6387417ee73..955429eaf125f 100644 --- a/site/src/pages/AuditPage/AuditPage.test.tsx +++ b/site/src/pages/AuditPage/AuditPage.test.tsx @@ -5,11 +5,14 @@ import { history, MockAuditLog, MockAuditLog2, + MockAuditLogWithEmptyDiff, render, waitForLoaderToBeRemoved, } from "testHelpers/renderHelpers" import * as CreateDayString from "util/createDayString" import AuditPage from "./AuditPage" +import { server } from "testHelpers/server" +import { rest } from "msw" describe("AuditPage", () => { beforeEach(() => { @@ -27,6 +30,21 @@ describe("AuditPage", () => { screen.getByTestId(`audit-log-row-${MockAuditLog2.id}`) }) + it("filters out audit logs with empty diffs", async () => { + server.use( + rest.get(`/api/v2/audit`, (_, res, ctx) => { + return res(ctx.status(200), ctx.json(MockAuditLogWithEmptyDiff)) + }), + ) + + // When + render() + + // Then + const logRow = screen.queryByTestId(`audit-log-row-${MockAuditLogWithEmptyDiff.id}`) + expect(logRow).not.toBeInTheDocument() + }) + describe("Filtering", () => { it("filters by typing", async () => { const getAuditLogsSpy = jest diff --git a/site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx b/site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx index b88f12ac2f2d6..abf446e64d09d 100644 --- a/site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx +++ b/site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx @@ -82,9 +82,10 @@ export const WorkspaceSchedulePage: React.FC = () => { navigate(`/@${username}/${workspaceName}`) }} onSubmit={(values) => { + console.log("values", values) scheduleSend({ type: "SUBMIT_SCHEDULE", - autoStart: formValuesToAutoStartRequest(values), + autoStart: values.autoStartEnabled ? formValuesToAutoStartRequest(values) : undefined, ttl: formValuesToTTLRequest(values), }) }} diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 36302613f30a6..eaf3a868fd7d0 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -829,7 +829,13 @@ export const MockAuditLog: TypesGen.AuditLog = { resource_target: "bruno-dev", resource_icon: "", action: "create", - diff: {}, + diff: { + ttl: { + old: 0, + new: 3600000000000, + secret: false, + }, + }, status_code: 200, additional_fields: "", description: "{user} updated workspace {target}", @@ -864,6 +870,26 @@ export const MockAuditLog2: TypesGen.AuditLog = { }, } +export const MockAuditLogWithEmptyDiff: TypesGen.AuditLog = { + id: "958d5bdc-d034-4857-891e-33393ba07e55", + request_id: "0585f890-2659-4761-bcb4-3bbca1fe4ab7", + time: "2022-10-12T13:36:31.682107Z", + organization_id: "fc0774ce-cc9e-48d4-80ae-88f7a4d4a8b0", + ip: "127.0.0.1", + user_agent: + '"Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/104.0.0.0 Safari/537.36"', + resource_type: "workspace", + resource_id: "ef8d1cf4-82de-4fd9-8980-047dad6d06b5", + resource_target: "bruno-dev", + resource_icon: "", + action: "create", + diff: {}, + status_code: 200, + additional_fields: "", + description: "{user} updated workspace {target}", + user: MockUser, +} + export const MockWorkspaceQuota: TypesGen.WorkspaceQuota = { user_workspace_count: 0, user_workspace_limit: 100, diff --git a/site/src/xServices/audit/auditXService.ts b/site/src/xServices/audit/auditXService.ts index 9c315bec2a860..e3a07805b1144 100644 --- a/site/src/xServices/audit/auditXService.ts +++ b/site/src/xServices/audit/auditXService.ts @@ -128,8 +128,10 @@ export const auditMachine = createMachine( }).then((data) => data.count), ]) + // Filter out any logs that don't have an actual diff to present to the user + const filteredLogs = auditLogs.filter((log) => Object.entries(log.diff).length > 0) return { - auditLogs, + auditLogs: filteredLogs, count, } }, diff --git a/site/src/xServices/workspaceSchedule/workspaceScheduleXService.ts b/site/src/xServices/workspaceSchedule/workspaceScheduleXService.ts index 67fa104736424..5ce4a176340d6 100644 --- a/site/src/xServices/workspaceSchedule/workspaceScheduleXService.ts +++ b/site/src/xServices/workspaceSchedule/workspaceScheduleXService.ts @@ -45,7 +45,7 @@ export type WorkspaceScheduleEvent = | { type: "GET_WORKSPACE"; username: string; workspaceName: string } | { type: "SUBMIT_SCHEDULE" - autoStart: TypesGen.UpdateWorkspaceAutostartRequest + autoStart: TypesGen.UpdateWorkspaceAutostartRequest | undefined ttl: TypesGen.UpdateWorkspaceTTLRequest } @@ -190,10 +190,9 @@ export const workspaceSchedule = createMachine( throw new Error("Failed to load workspace.") } - // REMARK: These calls are purposefully synchronous because if one - // value contradicts the other, we don't want a race condition - // on re-submission. - await API.putWorkspaceAutostart(context.workspace.id, event.autoStart) + if (event.autoStart?.schedule !== undefined) { + await API.putWorkspaceAutostart(context.workspace.id, event.autoStart) + } await API.putWorkspaceAutostop(context.workspace.id, event.ttl) }, }, From 3778a420f298c5210c4f214d0bcf37f1b8e6d288 Mon Sep 17 00:00:00 2001 From: Kira Pilot Date: Wed, 12 Oct 2022 14:19:53 +0000 Subject: [PATCH 2/5] remove console log --- site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx b/site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx index abf446e64d09d..164a61b6e401c 100644 --- a/site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx +++ b/site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx @@ -82,7 +82,6 @@ export const WorkspaceSchedulePage: React.FC = () => { navigate(`/@${username}/${workspaceName}`) }} onSubmit={(values) => { - console.log("values", values) scheduleSend({ type: "SUBMIT_SCHEDULE", autoStart: values.autoStartEnabled ? formValuesToAutoStartRequest(values) : undefined, From f344f0fa0b823837d18228da2e3a8699238179bc Mon Sep 17 00:00:00 2001 From: Kira Pilot Date: Wed, 12 Oct 2022 16:04:11 +0000 Subject: [PATCH 3/5] attempting to fix tests --- coderd/audit/audit.go | 8 ++++++-- site/src/pages/AuditPage/AuditPage.test.tsx | 4 +++- .../pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx | 4 +++- site/src/xServices/audit/auditXService.ts | 4 +++- 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/coderd/audit/audit.go b/coderd/audit/audit.go index 92f447130503c..d236bbe7c04a2 100644 --- a/coderd/audit/audit.go +++ b/coderd/audit/audit.go @@ -21,7 +21,9 @@ func (nop) Export(context.Context, database.AuditLog) error { return nil } -func (nop) diff(any, any) Map { return Map{} } +func (nop) diff(any, any) Map { + return Map{} +} func NewMock() *MockAuditor { return &MockAuditor{} @@ -36,4 +38,6 @@ func (a *MockAuditor) Export(_ context.Context, alog database.AuditLog) error { return nil } -func (*MockAuditor) diff(any, any) Map { return Map{} } +func (*MockAuditor) diff(any, any) Map { + return Map{"ttl": {"3600000000000", "7200000000000", false}} +} diff --git a/site/src/pages/AuditPage/AuditPage.test.tsx b/site/src/pages/AuditPage/AuditPage.test.tsx index e2feb9ee1000b..bd33c885c9ac6 100644 --- a/site/src/pages/AuditPage/AuditPage.test.tsx +++ b/site/src/pages/AuditPage/AuditPage.test.tsx @@ -41,7 +41,9 @@ describe("AuditPage", () => { render() // Then - const logRow = screen.queryByTestId(`audit-log-row-${MockAuditLogWithEmptyDiff.id}`) + const logRow = screen.queryByTestId( + `audit-log-row-${MockAuditLogWithEmptyDiff.id}`, + ) expect(logRow).not.toBeInTheDocument() }) diff --git a/site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx b/site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx index 63a36aa39b6ec..492a56ae47aa0 100644 --- a/site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx +++ b/site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx @@ -108,7 +108,9 @@ export const WorkspaceSchedulePage: React.FC = () => { onSubmit={(values) => { scheduleSend({ type: "SUBMIT_SCHEDULE", - autoStart: values.autoStartEnabled ? formValuesToAutoStartRequest(values) : undefined, + autoStart: values.autoStartEnabled + ? formValuesToAutoStartRequest(values) + : undefined, ttl: formValuesToTTLRequest(values), }) }} diff --git a/site/src/xServices/audit/auditXService.ts b/site/src/xServices/audit/auditXService.ts index 9ad8976a2bade..5063531ddc97a 100644 --- a/site/src/xServices/audit/auditXService.ts +++ b/site/src/xServices/audit/auditXService.ts @@ -132,7 +132,9 @@ export const auditMachine = createMachine( ]) // Filter out any logs that don't have an actual diff to present to the user - const filteredLogs = auditLogs.filter((log) => Object.entries(log.diff).length > 0) + const filteredLogs = auditLogs.filter( + (log) => Object.entries(log.diff).length > 0, + ) return { auditLogs: filteredLogs, count, From e259eba0b78fa6324c8ff71e75a806ca73042c0c Mon Sep 17 00:00:00 2001 From: Kira Pilot Date: Fri, 14 Oct 2022 18:20:48 +0000 Subject: [PATCH 4/5] keep diffs with 0 changes --- coderd/audit/audit.go | 2 +- coderd/audit/request.go | 4 ---- site/src/pages/AuditPage/AuditPage.test.tsx | 17 ----------------- site/src/testHelpers/entities.ts | 20 -------------------- site/src/xServices/audit/auditXService.ts | 6 +----- 5 files changed, 2 insertions(+), 47 deletions(-) diff --git a/coderd/audit/audit.go b/coderd/audit/audit.go index d236bbe7c04a2..ca343123e9716 100644 --- a/coderd/audit/audit.go +++ b/coderd/audit/audit.go @@ -39,5 +39,5 @@ func (a *MockAuditor) Export(_ context.Context, alog database.AuditLog) error { } func (*MockAuditor) diff(any, any) Map { - return Map{"ttl": {"3600000000000", "7200000000000", false}} + return Map{} } diff --git a/coderd/audit/request.go b/coderd/audit/request.go index a37ddd7e22224..c23ad2b1f7339 100644 --- a/coderd/audit/request.go +++ b/coderd/audit/request.go @@ -115,10 +115,6 @@ func InitRequest[T Auditable](w http.ResponseWriter, p *RequestParams) (*Request if sw.Status < 400 { diff := Diff(p.Audit, req.Old, req.New) - if len(diff) == 0 { - return - } - var err error diffRaw, err = json.Marshal(diff) if err != nil { diff --git a/site/src/pages/AuditPage/AuditPage.test.tsx b/site/src/pages/AuditPage/AuditPage.test.tsx index bd33c885c9ac6..4a30aca8f4fdb 100644 --- a/site/src/pages/AuditPage/AuditPage.test.tsx +++ b/site/src/pages/AuditPage/AuditPage.test.tsx @@ -30,23 +30,6 @@ describe("AuditPage", () => { screen.getByTestId(`audit-log-row-${MockAuditLog2.id}`) }) - it("filters out audit logs with empty diffs", async () => { - server.use( - rest.get(`/api/v2/audit`, (_, res, ctx) => { - return res(ctx.status(200), ctx.json(MockAuditLogWithEmptyDiff)) - }), - ) - - // When - render() - - // Then - const logRow = screen.queryByTestId( - `audit-log-row-${MockAuditLogWithEmptyDiff.id}`, - ) - expect(logRow).not.toBeInTheDocument() - }) - describe("Filtering", () => { it("filters by typing", async () => { const getAuditLogsSpy = jest diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 97b0799ababfc..9138debefcf3b 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -912,26 +912,6 @@ export const MockAuditLog2: TypesGen.AuditLog = { }, } -export const MockAuditLogWithEmptyDiff: TypesGen.AuditLog = { - id: "958d5bdc-d034-4857-891e-33393ba07e55", - request_id: "0585f890-2659-4761-bcb4-3bbca1fe4ab7", - time: "2022-10-12T13:36:31.682107Z", - organization_id: "fc0774ce-cc9e-48d4-80ae-88f7a4d4a8b0", - ip: "127.0.0.1", - user_agent: - '"Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/104.0.0.0 Safari/537.36"', - resource_type: "workspace", - resource_id: "ef8d1cf4-82de-4fd9-8980-047dad6d06b5", - resource_target: "bruno-dev", - resource_icon: "", - action: "create", - diff: {}, - status_code: 200, - additional_fields: "", - description: "{user} updated workspace {target}", - user: MockUser, -} - export const MockWorkspaceQuota: TypesGen.WorkspaceQuota = { user_workspace_count: 0, user_workspace_limit: 100, diff --git a/site/src/xServices/audit/auditXService.ts b/site/src/xServices/audit/auditXService.ts index 5063531ddc97a..318fb7b2865e1 100644 --- a/site/src/xServices/audit/auditXService.ts +++ b/site/src/xServices/audit/auditXService.ts @@ -131,12 +131,8 @@ export const auditMachine = createMachine( }).then((data) => data.count), ]) - // Filter out any logs that don't have an actual diff to present to the user - const filteredLogs = auditLogs.filter( - (log) => Object.entries(log.diff).length > 0, - ) return { - auditLogs: filteredLogs, + auditLogs, count, } }, From 72728af6c01e5c218818e9f6be06a0e49ea14e5a Mon Sep 17 00:00:00 2001 From: Kira Pilot Date: Fri, 14 Oct 2022 18:24:00 +0000 Subject: [PATCH 5/5] cleaned up test --- site/src/pages/AuditPage/AuditPage.test.tsx | 3 --- 1 file changed, 3 deletions(-) diff --git a/site/src/pages/AuditPage/AuditPage.test.tsx b/site/src/pages/AuditPage/AuditPage.test.tsx index 4a30aca8f4fdb..83c141fb2ac22 100644 --- a/site/src/pages/AuditPage/AuditPage.test.tsx +++ b/site/src/pages/AuditPage/AuditPage.test.tsx @@ -5,14 +5,11 @@ import { history, MockAuditLog, MockAuditLog2, - MockAuditLogWithEmptyDiff, render, waitForLoaderToBeRemoved, } from "testHelpers/renderHelpers" import * as CreateDayString from "util/createDayString" import AuditPage from "./AuditPage" -import { server } from "testHelpers/server" -import { rest } from "msw" describe("AuditPage", () => { beforeEach(() => {