Skip to content

Commit 7a1731b

Browse files
Kira-PilotKira-Pilot
authored
chore: change build audit log string to be clearer (#6093)
* changed bbuild string * clean up friendly string * using Trans component * general cleanup * fixed tests * fix lint * fixing bolding * removing dead strings in auditLogRow * fix tests
1 parent d60ec3e commit 7a1731b

14 files changed

+241
-144
lines changed

‎coderd/audit.go

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -244,13 +244,13 @@ func (api *API) convertAuditLog(ctx context.Context, dblog database.GetAuditLogs
244244
StatusCode: dblog.StatusCode,
245245
AdditionalFields: dblog.AdditionalFields,
246246
User: user,
247-
Description: auditLogDescription(dblog, additionalFields),
247+
Description: auditLogDescription(dblog),
248248
ResourceLink: resourceLink,
249249
IsDeleted: isDeleted,
250250
}
251251
}
252252

253-
func auditLogDescription(alog database.GetAuditLogsOffsetRow, additionalFields audit.AdditionalFields) string {
253+
func auditLogDescription(alog database.GetAuditLogsOffsetRow) string {
254254
str := fmt.Sprintf("{user} %s",
255255
codersdk.AuditAction(alog.Action).Friendly(),
256256
)
@@ -261,19 +261,6 @@ func auditLogDescription(alog database.GetAuditLogsOffsetRow, additionalFields a
261261
return str
262262
}
263263

264-
// Strings for starting/stopping workspace builds follow the below format:
265-
// "{user | 'Coder automatically'} started build #{build_number} for workspace {target}"
266-
// where target is a workspace (name) instead of a workspace build
267-
// passed in on the FE via AuditLog.AdditionalFields rather than derived in request.go:35
268-
if alog.ResourceType == database.ResourceTypeWorkspaceBuild && alog.Action != database.AuditActionDelete {
269-
if len(additionalFields.BuildNumber) == 0 {
270-
str += " build for"
271-
} else {
272-
str += fmt.Sprintf(" build #%s for",
273-
additionalFields.BuildNumber)
274-
}
275-
}
276-
277264
// We don't display the name (target) for git ssh keys. It's fairly long and doesn't
278265
// make too much sense to display.
279266
if alog.ResourceType == database.ResourceTypeGitSshKey {

‎site/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
"@xstate/inspect": "0.6.5",
4646
"@xstate/react": "3.0.1",
4747
"axios": "0.26.1",
48+
"canvas": "^2.11.0",
4849
"chart.js": "3.9.1",
4950
"chartjs-adapter-date-fns": "3.0.0",
5051
"color-convert": "2.0.1",
@@ -110,7 +111,6 @@
110111
"@typescript-eslint/eslint-plugin": "5.50.0",
111112
"@typescript-eslint/parser": "5.45.1",
112113
"@xstate/cli": "0.3.0",
113-
"canvas": "2.10.0",
114114
"chromatic": "6.15.0",
115115
"eslint": "8.33.0",
116116
"eslint-config-prettier": "8.5.0",

‎site/src/components/AuditLogRow/AuditLogDescription.tsx

Lines changed: 0 additions & 80 deletions
This file was deleted.

‎site/src/components/AuditLogRow/AuditLogDescription.test.tsx renamed to ‎site/src/components/AuditLogRow/AuditLogDescription/AuditLogDescription.test.tsx

Lines changed: 56 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,13 @@ import {
77
MockAuditLogUnsuccessfulLoginUnknownUser,
88
} from "testHelpers/entities"
99
import { AuditLogDescription } from "./AuditLogDescription"
10-
import { AuditLogRow } from "./AuditLogRow"
11-
import { render } from "../../testHelpers/renderHelpers"
10+
import { AuditLogRow } from "../AuditLogRow"
11+
import { render } from "testHelpers/renderHelpers"
1212
import { screen } from "@testing-library/react"
13+
import { i18n } from "i18n"
14+
15+
const t = (str: string, variables?: Record<string, unknown>) =>
16+
i18n.t<string>(str, variables)
1317

1418
const getByTextContent = (text: string) => {
1519
return screen.getByText((_, element) => {
@@ -25,17 +29,14 @@ describe("AuditLogDescription", () => {
2529
it("renders the correct string for a workspace create audit log", async () => {
2630
render(<AuditLogDescription auditLog={MockAuditLog} />)
2731

28-
expect(
29-
getByTextContent("TestUser created workspace bruno-dev"),
30-
).toBeDefined()
32+
expect(screen.getByText("TestUser created workspace")).toBeDefined()
33+
expect(screen.getByText("bruno-dev")).toBeDefined()
3134
})
3235

3336
it("renders the correct string for a workspace_build stop audit log", async () => {
3437
render(<AuditLogDescription auditLog={MockAuditLogWithWorkspaceBuild} />)
3538

36-
expect(
37-
getByTextContent("TestUser stopped build for workspace test2"),
38-
).toBeDefined()
39+
expect(getByTextContent("TestUser stopped workspace test2")).toBeDefined()
3940
})
4041

4142
it("renders the correct string for a workspace_build audit log with a duplicate word", async () => {
@@ -48,7 +49,7 @@ describe("AuditLogDescription", () => {
4849
render(<AuditLogDescription auditLog={AuditLogWithRepeat} />)
4950

5051
expect(
51-
getByTextContent("TestUser stopped build for workspace workspace"),
52+
getByTextContent("TestUser stopped workspace workspace"),
5253
).toBeDefined()
5354
})
5455
it("renders the correct string for a workspace created for a different owner", async () => {
@@ -57,27 +58,68 @@ describe("AuditLogDescription", () => {
5758
auditLog={MockWorkspaceCreateAuditLogForDifferentOwner}
5859
/>,
5960
)
61+
6062
expect(
61-
getByTextContent(
62-
`TestUser created workspace bruno-dev on behalf of ${MockWorkspaceCreateAuditLogForDifferentOwner.additional_fields.workspace_owner}`,
63+
screen.getByText(
64+
`on behalf of ${MockWorkspaceCreateAuditLogForDifferentOwner.additional_fields.workspace_owner}`,
65+
{ exact: false },
6366
),
6467
).toBeDefined()
6568
})
6669
it("renders the correct string for successful login", async () => {
6770
render(<AuditLogRow auditLog={MockAuditLogSuccessfulLogin} />)
68-
expect(getByTextContent(`TestUser logged in`)).toBeDefined()
71+
72+
expect(
73+
screen.getByText(
74+
t("auditLog:table.logRow.description.unlinkedAuditDescription", {
75+
truncatedDescription: `${MockAuditLogSuccessfulLogin.user?.username} logged in`,
76+
target: "",
77+
onBehalfOf: undefined,
78+
})
79+
.replace(/<[^>]*>/g, " ")
80+
.replace(/\s{2,}/g, " ")
81+
.trim(),
82+
),
83+
).toBeInTheDocument()
84+
6985
const statusPill = screen.getByRole("status")
7086
expect(statusPill).toHaveTextContent("201")
7187
})
7288
it("renders the correct string for unsuccessful login for a known user", async () => {
7389
render(<AuditLogRow auditLog={MockAuditLogUnsuccessfulLoginKnownUser} />)
74-
expect(getByTextContent(`TestUser logged in`)).toBeDefined()
90+
91+
expect(
92+
screen.getByText(
93+
t("auditLog:table.logRow.description.unlinkedAuditDescription", {
94+
truncatedDescription: `${MockAuditLogUnsuccessfulLoginKnownUser.user?.username} logged in`,
95+
target: "",
96+
onBehalfOf: undefined,
97+
})
98+
.replace(/<[^>]*>/g, " ")
99+
.replace(/\s{2,}/g, " ")
100+
.trim(),
101+
),
102+
).toBeInTheDocument()
103+
75104
const statusPill = screen.getByRole("status")
76105
expect(statusPill).toHaveTextContent("401")
77106
})
78107
it("renders the correct string for unsuccessful login for an unknown user", async () => {
79108
render(<AuditLogRow auditLog={MockAuditLogUnsuccessfulLoginUnknownUser} />)
80-
expect(getByTextContent(`an unknown user logged in`)).toBeDefined()
109+
110+
expect(
111+
screen.getByText(
112+
t("auditLog:table.logRow.description.unlinkedAuditDescription", {
113+
truncatedDescription: "an unknown user logged in",
114+
target: "",
115+
onBehalfOf: undefined,
116+
})
117+
.replace(/<[^>]*>/g, " ")
118+
.replace(/\s{2,}/g, " ")
119+
.trim(),
120+
),
121+
).toBeInTheDocument()
122+
81123
const statusPill = screen.getByRole("status")
82124
expect(statusPill).toHaveTextContent("401")
83125
})

‎site/src/components/AuditLogRow/AuditLogDescription/AuditLogDescription.tsx

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
import { FC } from "react"
2+
import { AuditLog } from "api/typesGenerated"
3+
import { Link as RouterLink } from "react-router-dom"
4+
import Link from "@material-ui/core/Link"
5+
import { Trans, useTranslation } from "react-i18next"
6+
import { BuildAuditDescription } from "./BuildAuditDescription"
7+
8+
export const AuditLogDescription: FC<{ auditLog: AuditLog }> = ({
9+
auditLog,
10+
}): JSX.Element => {
11+
const { t } = useTranslation("auditLog")
12+
13+
let target = auditLog.resource_target.trim()
14+
const user = auditLog.user ? auditLog.user.username.trim() : "an unknown user"
15+
16+
if (auditLog.resource_type === "workspace_build") {
17+
return <BuildAuditDescription auditLog={auditLog} />
18+
}
19+
20+
// SSH key entries have no links
21+
if (auditLog.resource_type === "git_ssh_key") {
22+
target = ""
23+
}
24+
25+
const truncatedDescription = auditLog.description
26+
.replace("{user}", `${user}`)
27+
.replace("{target}", "")
28+
29+
// logs for workspaces created on behalf of other users indicate ownership in the description
30+
const onBehalfOf =
31+
auditLog.additional_fields.workspace_owner &&
32+
auditLog.additional_fields.workspace_owner !== "unknown" &&
33+
auditLog.additional_fields.workspace_owner !== auditLog.user?.username
34+
? `on behalf of ${auditLog.additional_fields.workspace_owner}`
35+
: ""
36+
37+
if (auditLog.resource_link) {
38+
return (
39+
<span>
40+
<Trans
41+
t={t}
42+
i18nKey="table.logRow.description.linkedAuditDescription"
43+
values={{ truncatedDescription, target, onBehalfOf }}
44+
>
45+
{"{{truncatedDescription}}"}
46+
<Link component={RouterLink} to={auditLog.resource_link}>
47+
<strong>{"{{target}}"}</strong>
48+
</Link>
49+
{"{{onBehalfOf}}"}
50+
</Trans>
51+
</span>
52+
)
53+
}
54+
55+
return (
56+
<span>
57+
<Trans
58+
t={t}
59+
i18nKey="table.logRow.description.unlinkedAuditDescription"
60+
values={{ truncatedDescription, target, onBehalfOf }}
61+
>
62+
{"{{truncatedDescription}}"}
63+
<strong>{"{{target}}"}</strong>
64+
{"{{onBehalfOf}}"}
65+
</Trans>
66+
</span>
67+
)
68+
}

‎site/src/components/AuditLogRow/AuditLogDescription/BuildAuditDescription.tsx

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
import { Trans, useTranslation } from "react-i18next"
2+
import { AuditLog } from "api/typesGenerated"
3+
import { FC } from "react"
4+
import { Link as RouterLink } from "react-router-dom"
5+
import Link from "@material-ui/core/Link"
6+
7+
export const BuildAuditDescription: FC<{ auditLog: AuditLog }> = ({
8+
auditLog,
9+
}): JSX.Element => {
10+
const { t } = useTranslation("auditLog")
11+
12+
const workspaceName = auditLog.additional_fields?.workspace_name?.trim()
13+
// workspaces can be started/stopped by a user, or kicked off automatically by Coder
14+
const user =
15+
auditLog.additional_fields?.build_reason &&
16+
auditLog.additional_fields?.build_reason !== "initiator"
17+
? "Coder automatically"
18+
: auditLog.user?.username.trim()
19+
20+
const action = auditLog.action === "start" ? "started" : "stopped"
21+
22+
if (auditLog.resource_link) {
23+
return (
24+
<span>
25+
<Trans
26+
t={t}
27+
i18nKey="table.logRow.description.linkedWorkspaceBuild"
28+
values={{ user, action, workspaceName }}
29+
>
30+
{"{{user}}"}
31+
<Link component={RouterLink} to={auditLog.resource_link}>
32+
{"{{action}}"}
33+
</Link>
34+
workspace{"{{workspaceName}}"}
35+
</Trans>
36+
</span>
37+
)
38+
}
39+
40+
return (
41+
<span>
42+
<Trans
43+
t={t}
44+
i18nKey="table.logRow.description.unlinkedWorkspaceBuild"
45+
values={{ user, action, workspaceName }}
46+
>
47+
{"{{user}}"}
48+
{"{{action}}"}workspace{"{{workspaceName}}"}
49+
</Trans>
50+
</span>
51+
)
52+
}

‎site/src/components/AuditLogRow/AuditLogDescription/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export { AuditLogDescription } from "./AuditLogDescription"

‎site/src/components/AuditLogRow/AuditLogDiff.tsx renamed to ‎site/src/components/AuditLogRow/AuditLogDiff/AuditLogDiff.tsx

File renamed without changes.

‎site/src/components/AuditLogRow/auditUtils.test.ts renamed to ‎site/src/components/AuditLogRow/AuditLogDiff/auditUtils.test.ts

File renamed without changes.

‎site/src/components/AuditLogRow/auditUtils.ts renamed to ‎site/src/components/AuditLogRow/AuditLogDiff/auditUtils.ts

File renamed without changes.

‎site/src/components/AuditLogRow/AuditLogDiff/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
export { AuditLogDiff } from "./AuditLogDiff"
2+
export { determineGroupDiff } from "./auditUtils"

0 commit comments

Comments
 (0)