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
This commit is contained in:
Kira Pilot 2023-02-08 13:06:57 -05:00 committed by GitHub
parent d60ec3e4bf
commit 7a1731b620
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 241 additions and 144 deletions

View File

@ -244,13 +244,13 @@ func (api *API) convertAuditLog(ctx context.Context, dblog database.GetAuditLogs
StatusCode: dblog.StatusCode,
AdditionalFields: dblog.AdditionalFields,
User: user,
Description: auditLogDescription(dblog, additionalFields),
Description: auditLogDescription(dblog),
ResourceLink: resourceLink,
IsDeleted: isDeleted,
}
}
func auditLogDescription(alog database.GetAuditLogsOffsetRow, additionalFields audit.AdditionalFields) string {
func auditLogDescription(alog database.GetAuditLogsOffsetRow) string {
str := fmt.Sprintf("{user} %s",
codersdk.AuditAction(alog.Action).Friendly(),
)
@ -261,19 +261,6 @@ func auditLogDescription(alog database.GetAuditLogsOffsetRow, additionalFields a
return str
}
// Strings for starting/stopping workspace builds follow the below format:
// "{user | 'Coder automatically'} started build #{build_number} for workspace {target}"
// where target is a workspace (name) instead of a workspace build
// passed in on the FE via AuditLog.AdditionalFields rather than derived in request.go:35
if alog.ResourceType == database.ResourceTypeWorkspaceBuild && alog.Action != database.AuditActionDelete {
if len(additionalFields.BuildNumber) == 0 {
str += " build for"
} else {
str += fmt.Sprintf(" build #%s for",
additionalFields.BuildNumber)
}
}
// We don't display the name (target) for git ssh keys. It's fairly long and doesn't
// make too much sense to display.
if alog.ResourceType == database.ResourceTypeGitSshKey {

View File

@ -45,6 +45,7 @@
"@xstate/inspect": "0.6.5",
"@xstate/react": "3.0.1",
"axios": "0.26.1",
"canvas": "^2.11.0",
"chart.js": "3.9.1",
"chartjs-adapter-date-fns": "3.0.0",
"color-convert": "2.0.1",
@ -110,7 +111,6 @@
"@typescript-eslint/eslint-plugin": "5.50.0",
"@typescript-eslint/parser": "5.45.1",
"@xstate/cli": "0.3.0",
"canvas": "2.10.0",
"chromatic": "6.15.0",
"eslint": "8.33.0",
"eslint-config-prettier": "8.5.0",

View File

@ -1,80 +0,0 @@
import { FC } from "react"
import { AuditLog } from "api/typesGenerated"
import { Link as RouterLink } from "react-router-dom"
import Link from "@material-ui/core/Link"
import { makeStyles } from "@material-ui/core/styles"
import i18next from "i18next"
export const AuditLogDescription: FC<{ auditLog: AuditLog }> = ({
auditLog,
}): JSX.Element => {
const classes = useStyles()
const { t } = i18next
let target = auditLog.resource_target.trim()
let user = auditLog.user
? auditLog.user.username.trim()
: t("auditLog:table.logRow.unknownUser")
if (auditLog.resource_type === "workspace_build") {
// audit logs with a resource_type of workspace build use workspace name as a target
target = auditLog.additional_fields?.workspace_name?.trim()
// workspaces can be started/stopped by a user, or kicked off automatically by Coder
user =
auditLog.additional_fields?.build_reason &&
auditLog.additional_fields?.build_reason !== "initiator"
? t("auditLog:table.logRow.buildReason")
: user
}
// SSH key entries have no links
if (auditLog.resource_type === "git_ssh_key") {
return (
<span>
{auditLog.description
.replace("{user}", `${auditLog.user?.username.trim()}`)
.replace("{target}", `${target}`)}
</span>
)
}
const truncatedDescription = auditLog.description
.replace("{user}", `${user}`)
.replace("{target}", "")
return (
<span>
{truncatedDescription}
{auditLog.resource_link ? (
<Link component={RouterLink} to={auditLog.resource_link}>
<strong>{target}</strong>
</Link>
) : (
<strong>{target}</strong>
)}
{auditLog.is_deleted && (
<span className={classes.deletedLabel}>
<>{t("auditLog:table.logRow.deletedLabel")}</>
</span>
)}
{/* logs for workspaces created on behalf of other users indicate ownership in the description */}
{auditLog.additional_fields.workspace_owner &&
auditLog.additional_fields.workspace_owner !== "unknown" && (
<span>
<>
{t("auditLog:table.logRow.onBehalfOf", {
owner: auditLog.additional_fields.workspace_owner,
})}
</>
</span>
)}
</span>
)
}
const useStyles = makeStyles((theme) => ({
deletedLabel: {
...theme.typography.caption,
color: theme.palette.text.secondary,
},
}))

View File

@ -7,9 +7,13 @@ import {
MockAuditLogUnsuccessfulLoginUnknownUser,
} from "testHelpers/entities"
import { AuditLogDescription } from "./AuditLogDescription"
import { AuditLogRow } from "./AuditLogRow"
import { render } from "../../testHelpers/renderHelpers"
import { AuditLogRow } from "../AuditLogRow"
import { render } from "testHelpers/renderHelpers"
import { screen } from "@testing-library/react"
import { i18n } from "i18n"
const t = (str: string, variables?: Record<string, unknown>) =>
i18n.t<string>(str, variables)
const getByTextContent = (text: string) => {
return screen.getByText((_, element) => {
@ -25,17 +29,14 @@ describe("AuditLogDescription", () => {
it("renders the correct string for a workspace create audit log", async () => {
render(<AuditLogDescription auditLog={MockAuditLog} />)
expect(
getByTextContent("TestUser created workspace bruno-dev"),
).toBeDefined()
expect(screen.getByText("TestUser created workspace")).toBeDefined()
expect(screen.getByText("bruno-dev")).toBeDefined()
})
it("renders the correct string for a workspace_build stop audit log", async () => {
render(<AuditLogDescription auditLog={MockAuditLogWithWorkspaceBuild} />)
expect(
getByTextContent("TestUser stopped build for workspace test2"),
).toBeDefined()
expect(getByTextContent("TestUser stopped workspace test2")).toBeDefined()
})
it("renders the correct string for a workspace_build audit log with a duplicate word", async () => {
@ -48,7 +49,7 @@ describe("AuditLogDescription", () => {
render(<AuditLogDescription auditLog={AuditLogWithRepeat} />)
expect(
getByTextContent("TestUser stopped build for workspace workspace"),
getByTextContent("TestUser stopped workspace workspace"),
).toBeDefined()
})
it("renders the correct string for a workspace created for a different owner", async () => {
@ -57,27 +58,68 @@ describe("AuditLogDescription", () => {
auditLog={MockWorkspaceCreateAuditLogForDifferentOwner}
/>,
)
expect(
getByTextContent(
`TestUser created workspace bruno-dev on behalf of ${MockWorkspaceCreateAuditLogForDifferentOwner.additional_fields.workspace_owner}`,
screen.getByText(
`on behalf of ${MockWorkspaceCreateAuditLogForDifferentOwner.additional_fields.workspace_owner}`,
{ exact: false },
),
).toBeDefined()
})
it("renders the correct string for successful login", async () => {
render(<AuditLogRow auditLog={MockAuditLogSuccessfulLogin} />)
expect(getByTextContent(`TestUser logged in`)).toBeDefined()
expect(
screen.getByText(
t("auditLog:table.logRow.description.unlinkedAuditDescription", {
truncatedDescription: `${MockAuditLogSuccessfulLogin.user?.username} logged in`,
target: "",
onBehalfOf: undefined,
})
.replace(/<[^>]*>/g, " ")
.replace(/\s{2,}/g, " ")
.trim(),
),
).toBeInTheDocument()
const statusPill = screen.getByRole("status")
expect(statusPill).toHaveTextContent("201")
})
it("renders the correct string for unsuccessful login for a known user", async () => {
render(<AuditLogRow auditLog={MockAuditLogUnsuccessfulLoginKnownUser} />)
expect(getByTextContent(`TestUser logged in`)).toBeDefined()
expect(
screen.getByText(
t("auditLog:table.logRow.description.unlinkedAuditDescription", {
truncatedDescription: `${MockAuditLogUnsuccessfulLoginKnownUser.user?.username} logged in`,
target: "",
onBehalfOf: undefined,
})
.replace(/<[^>]*>/g, " ")
.replace(/\s{2,}/g, " ")
.trim(),
),
).toBeInTheDocument()
const statusPill = screen.getByRole("status")
expect(statusPill).toHaveTextContent("401")
})
it("renders the correct string for unsuccessful login for an unknown user", async () => {
render(<AuditLogRow auditLog={MockAuditLogUnsuccessfulLoginUnknownUser} />)
expect(getByTextContent(`an unknown user logged in`)).toBeDefined()
expect(
screen.getByText(
t("auditLog:table.logRow.description.unlinkedAuditDescription", {
truncatedDescription: "an unknown user logged in",
target: "",
onBehalfOf: undefined,
})
.replace(/<[^>]*>/g, " ")
.replace(/\s{2,}/g, " ")
.trim(),
),
).toBeInTheDocument()
const statusPill = screen.getByRole("status")
expect(statusPill).toHaveTextContent("401")
})

View File

@ -0,0 +1,68 @@
import { FC } from "react"
import { AuditLog } from "api/typesGenerated"
import { Link as RouterLink } from "react-router-dom"
import Link from "@material-ui/core/Link"
import { Trans, useTranslation } from "react-i18next"
import { BuildAuditDescription } from "./BuildAuditDescription"
export const AuditLogDescription: FC<{ auditLog: AuditLog }> = ({
auditLog,
}): JSX.Element => {
const { t } = useTranslation("auditLog")
let target = auditLog.resource_target.trim()
const user = auditLog.user ? auditLog.user.username.trim() : "an unknown user"
if (auditLog.resource_type === "workspace_build") {
return <BuildAuditDescription auditLog={auditLog} />
}
// SSH key entries have no links
if (auditLog.resource_type === "git_ssh_key") {
target = ""
}
const truncatedDescription = auditLog.description
.replace("{user}", `${user}`)
.replace("{target}", "")
// logs for workspaces created on behalf of other users indicate ownership in the description
const onBehalfOf =
auditLog.additional_fields.workspace_owner &&
auditLog.additional_fields.workspace_owner !== "unknown" &&
auditLog.additional_fields.workspace_owner !== auditLog.user?.username
? `on behalf of ${auditLog.additional_fields.workspace_owner}`
: ""
if (auditLog.resource_link) {
return (
<span>
<Trans
t={t}
i18nKey="table.logRow.description.linkedAuditDescription"
values={{ truncatedDescription, target, onBehalfOf }}
>
{"{{truncatedDescription}}"}
<Link component={RouterLink} to={auditLog.resource_link}>
<strong>{"{{target}}"}</strong>
</Link>
{"{{onBehalfOf}}"}
</Trans>
</span>
)
}
return (
<span>
<Trans
t={t}
i18nKey="table.logRow.description.unlinkedAuditDescription"
values={{ truncatedDescription, target, onBehalfOf }}
>
{"{{truncatedDescription}}"}
<strong>{"{{target}}"}</strong>
{"{{onBehalfOf}}"}
</Trans>
</span>
)
}

View File

@ -0,0 +1,52 @@
import { Trans, useTranslation } from "react-i18next"
import { AuditLog } from "api/typesGenerated"
import { FC } from "react"
import { Link as RouterLink } from "react-router-dom"
import Link from "@material-ui/core/Link"
export const BuildAuditDescription: FC<{ auditLog: AuditLog }> = ({
auditLog,
}): JSX.Element => {
const { t } = useTranslation("auditLog")
const workspaceName = auditLog.additional_fields?.workspace_name?.trim()
// workspaces can be started/stopped by a user, or kicked off automatically by Coder
const user =
auditLog.additional_fields?.build_reason &&
auditLog.additional_fields?.build_reason !== "initiator"
? "Coder automatically"
: auditLog.user?.username.trim()
const action = auditLog.action === "start" ? "started" : "stopped"
if (auditLog.resource_link) {
return (
<span>
<Trans
t={t}
i18nKey="table.logRow.description.linkedWorkspaceBuild"
values={{ user, action, workspaceName }}
>
{"{{user}}"}
<Link component={RouterLink} to={auditLog.resource_link}>
{"{{action}}"}
</Link>
workspace{"{{workspaceName}}"}
</Trans>
</span>
)
}
return (
<span>
<Trans
t={t}
i18nKey="table.logRow.description.unlinkedWorkspaceBuild"
values={{ user, action, workspaceName }}
>
{"{{user}}"}
{"{{action}}"}workspace{"{{workspaceName}}"}
</Trans>
</span>
)
}

View File

@ -0,0 +1 @@
export { AuditLogDescription } from "./AuditLogDescription"

View File

@ -0,0 +1,2 @@
export { AuditLogDiff } from "./AuditLogDiff"
export { determineGroupDiff } from "./auditUtils"

View File

@ -13,10 +13,9 @@ import { UserAvatar } from "components/UserAvatar/UserAvatar"
import { useState } from "react"
import { PaletteIndex } from "theme/palettes"
import userAgentParser from "ua-parser-js"
import { AuditLogDiff } from "./AuditLogDiff"
import i18next from "i18next"
import { AuditLogDiff, determineGroupDiff } from "./AuditLogDiff"
import { useTranslation } from "react-i18next"
import { AuditLogDescription } from "./AuditLogDescription"
import { determineGroupDiff } from "./auditUtils"
const httpStatusColor = (httpStatus: number): PaletteIndex => {
// redirects are successful
@ -46,14 +45,11 @@ export const AuditLogRow: React.FC<AuditLogRowProps> = ({
defaultIsDiffOpen = false,
}) => {
const styles = useStyles()
const { t } = i18next
const { t } = useTranslation("auditLog")
const [isDiffOpen, setIsDiffOpen] = useState(defaultIsDiffOpen)
const diffs = Object.entries(auditLog.diff)
const shouldDisplayDiff = diffs.length > 0
const { os, browser } = userAgentParser(auditLog.user_agent)
const displayBrowserInfo = browser.name
? `${browser.name} ${browser.version}`
: t("auditLog:table.logRow.notAvailable")
let auditDiff = auditLog.diff
@ -115,6 +111,11 @@ export const AuditLogRow: React.FC<AuditLogRowProps> = ({
spacing={1}
>
<AuditLogDescription auditLog={auditLog} />
{auditLog.is_deleted && (
<span className={styles.deletedLabel}>
<>{t("table.logRow.deletedLabel")}</>
</span>
)}
<span className={styles.auditLogTime}>
{new Date(auditLog.time).toLocaleTimeString()}
</span>
@ -124,25 +125,24 @@ export const AuditLogRow: React.FC<AuditLogRowProps> = ({
<Stack direction="row" spacing={1} alignItems="baseline">
{auditLog.ip && (
<span className={styles.auditLogInfo}>
<>{t("auditLog:table.logRow.ip")}</>
<>{t("table.logRow.ip")}</>
<strong>{auditLog.ip}</strong>
</span>
)}
<span className={styles.auditLogInfo}>
<>{t("auditLog:table.logRow.os")}</>
<strong>
{os.name
? os.name
: // https://github.com/i18next/next-i18next/issues/1795
t<string>("auditLog:table.logRow.notAvailable")}
</strong>
</span>
<span className={styles.auditLogInfo}>
<>{t("auditLog:table.logRow.browser")}</>
<strong>{displayBrowserInfo}</strong>
</span>
{os.name && (
<span className={styles.auditLogInfo}>
<>{t("table.logRow.os")}</>
<strong>{os.name}</strong>
</span>
)}
{browser.name && (
<span className={styles.auditLogInfo}>
<>{t("table.logRow.browser")}</>
<strong>
{browser.name} {browser.version}
</strong>
</span>
)}
</Stack>
<Pill
@ -220,4 +220,9 @@ const useStyles = makeStyles((theme) => ({
paddingRight: 10,
fontWeight: 600,
},
deletedLabel: {
...theme.typography.caption,
color: theme.palette.text.secondary,
},
}))

View File

@ -8,13 +8,16 @@
"emptyPage": "No audit logs available on this page",
"noLogs": "No audit logs available",
"logRow": {
"description": {
"linkedWorkspaceBuild": "{{user}} <1>{{action}}</1> workspace <strong>{{workspaceName}}</strong>",
"unlinkedWorkspaceBuild": "{{user}} {{action}} workspace <strong>{{workspaceName}}</strong>",
"linkedAuditDescription": "{{truncatedDescription}} <1><strong>{{target}}</strong></1> {{onBehalfOf}}",
"unlinkedAuditDescription": "{{truncatedDescription}} <strong>{{target}}</strong> {{onBehalfOf}}"
},
"deletedLabel": " (deleted)",
"ip": "IP: ",
"os": "OS: ",
"browser": "Browser: ",
"onBehalfOf": " on behalf of {{owner}}",
"buildReason": "Coder automatically",
"unknownUser": "an unknown user"
"browser": "Browser: "
}
},
"paywall": {

View File

@ -5170,13 +5170,13 @@ caniuse-lite@^1.0.30001109, caniuse-lite@^1.0.30001304, caniuse-lite@^1.0.300014
resolved "https://registry.yarnpkg.com/caniuse-lite/-/caniuse-lite-1.0.30001431.tgz#e7c59bd1bc518fae03a4656be442ce6c4887a795"
integrity sha512-zBUoFU0ZcxpvSt9IU66dXVT/3ctO1cy4y9cscs1szkPlcWb6pasYM144GqrUygUbT+k7cmUCW61cvskjcv0enQ==
canvas@2.10.0:
version "2.10.0"
resolved "https://registry.yarnpkg.com/canvas/-/canvas-2.10.0.tgz#5f48c8d1ff86c96356809097020336c3a1ccce27"
integrity sha512-A0RPxLcH0pPKAY2VN243LdCNcOJXAT8n7nJnN7TZMGv9OuF8we9wfpWgVT/eFMzi+cDYf/384w4BpfjGCD9aKQ==
canvas@^2.11.0:
version "2.11.0"
resolved "https://registry.yarnpkg.com/canvas/-/canvas-2.11.0.tgz#7f0c3e9ae94cf469269b5d3a7963a7f3a9936434"
integrity sha512-bdTjFexjKJEwtIo0oRx8eD4G2yWoUOXP9lj279jmQ2zMnTQhT8C3512OKz3s+ZOaQlLbE7TuVvRDYDB3Llyy5g==
dependencies:
"@mapbox/node-pre-gyp" "^1.0.0"
nan "^2.15.0"
nan "^2.17.0"
simple-get "^3.0.3"
capture-exit@^2.0.0:
@ -10788,6 +10788,11 @@ minipass@^3.0.0, minipass@^3.1.1:
dependencies:
yallist "^4.0.0"
minipass@^4.0.0:
version "4.0.2"
resolved "https://registry.yarnpkg.com/minipass/-/minipass-4.0.2.tgz#26fc3364d5ea6cb971c6e5259eac67a0887510d1"
integrity sha512-4Hbzei7ZyBp+1aw0874YWpKOubZd/jc53/XU+gkYry1QV+VvrbO8icLM5CUtm4F0hyXn85DXYKEMIS26gitD3A==
minizlib@^2.1.1:
version "2.1.2"
resolved "https://registry.yarnpkg.com/minizlib/-/minizlib-2.1.2.tgz#e90d3466ba209b932451508a11ce3d3632145931"
@ -10917,7 +10922,7 @@ mute-stream@0.0.8:
resolved "https://registry.yarnpkg.com/mute-stream/-/mute-stream-0.0.8.tgz#1630c42b2251ff81e2a283de96a5497ea92e5e0d"
integrity sha512-nnbWWOkoWyUsTjKrhgD0dcz22mdkSnpYqbEjIm2nhwhuxlSkpywJmBo8h0ZqJdkp73mb90SssHkN4rsRaBAfAA==
nan@^2.12.1, nan@^2.15.0:
nan@^2.12.1, nan@^2.17.0:
version "2.17.0"
resolved "https://registry.yarnpkg.com/nan/-/nan-2.17.0.tgz#c0150a2368a182f033e9aa5195ec76ea41a199cb"
integrity sha512-2ZTgtl0nJsO0KQCjEpxcIr5D+Yv90plTitZt9JBfQvVJDS5seMl3FOvsh3+9CoYWXf/1l5OaZzzF6nDm4cagaQ==
@ -13689,7 +13694,7 @@ tar-stream@^2.0.1:
inherits "^2.0.3"
readable-stream "^3.1.1"
tar@^6.0.2, tar@^6.1.11:
tar@^6.0.2:
version "6.1.12"
resolved "https://registry.yarnpkg.com/tar/-/tar-6.1.12.tgz#3b742fb05669b55671fb769ab67a7791ea1a62e6"
integrity sha512-jU4TdemS31uABHd+Lt5WEYJuzn+TJTCBLljvIAHZOz6M9Os5pJ4dD+vRFLxPa/n3T0iEFzpi+0x1UfuDZYbRMw==
@ -13701,6 +13706,18 @@ tar@^6.0.2, tar@^6.1.11:
mkdirp "^1.0.3"
yallist "^4.0.0"
tar@^6.1.11:
version "6.1.13"
resolved "https://registry.yarnpkg.com/tar/-/tar-6.1.13.tgz#46e22529000f612180601a6fe0680e7da508847b"
integrity sha512-jdIBIN6LTIe2jqzay/2vtYLlBHa3JF42ot3h1dW8Q0PaAG4v8rm0cvpVePtau5C6OKXGGcgO9q2AMNSWxiLqKw==
dependencies:
chownr "^2.0.0"
fs-minipass "^2.0.0"
minipass "^4.0.0"
minizlib "^2.1.1"
mkdirp "^1.0.3"
yallist "^4.0.0"
telejson@^6.0.8:
version "6.0.8"
resolved "https://registry.yarnpkg.com/telejson/-/telejson-6.0.8.tgz#1c432db7e7a9212c1fbd941c3e5174ec385148f7"