Skip to content

Fix access list audit log formatting#33344

Merged
kimlisa merged 3 commits intomasterfrom
lisa/fix-audit-log-formatting
Oct 12, 2023
Merged

Fix access list audit log formatting#33344
kimlisa merged 3 commits intomasterfrom
lisa/fix-audit-log-formatting

Conversation

@kimlisa
Copy link
Copy Markdown
Contributor

@kimlisa kimlisa commented Oct 11, 2023

from :
image

fixed:
image

return str.substring(0, len - 3) + '...';
}

function formattedMembersTxt(members: { member_name: string }[]) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit shorter option:

Suggested change
function formattedMembersTxt(members: { member_name: string }[]) {
function formatMembers(members: { member_name: string }[]) {

@@ -3062,6 +3062,7 @@ export const events = [
updated_by: 'mike',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we remove updated_by field from members in fixtures? RawEventAccessList.members doesn't contain it.

Comment on lines +1595 to +1599
if (memberNames.length > 1) {
return `members [${memberNamesJoined}]`;
}

return `member [${memberNamesJoined}]`;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import { pluralize } from 'shared/utils/text';
Suggested change
if (memberNames.length > 1) {
return `members [${memberNamesJoined}]`;
}
return `member [${memberNamesJoined}]`;
return `${pluralize(memberNames.length, "member")} [${memberNamesJoined}]`

😎

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a fixture with multiple members?

@public-teleport-github-review-bot public-teleport-github-review-bot Bot removed the request for review from mdwn October 12, 2023 13:11
Copy link
Copy Markdown
Contributor

@mdwn mdwn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this!

@kimlisa kimlisa enabled auto-merge October 12, 2023 17:38
@kimlisa kimlisa added this pull request to the merge queue Oct 12, 2023
Merged via the queue into master with commit 74bb298 Oct 12, 2023
@kimlisa kimlisa deleted the lisa/fix-audit-log-formatting branch October 12, 2023 18:03
@public-teleport-github-review-bot
Copy link
Copy Markdown

@kimlisa See the table below for backport results.

Branch Result
branch/v13 Create PR
branch/v14 Create PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants