Skip to content

feat(site): add new filter to audit logs#7878

Merged
BrunoQuaresma merged 7 commits into
mainfrom
bq/add-filter-to-audit-logs
Jun 7, 2023
Merged

feat(site): add new filter to audit logs#7878
BrunoQuaresma merged 7 commits into
mainfrom
bq/add-filter-to-audit-logs

Conversation

@BrunoQuaresma

Copy link
Copy Markdown
Contributor
Screen.Recording.2023-06-06.at.15.46.06.mov

@BrunoQuaresma BrunoQuaresma requested a review from Kira-Pilot June 6, 2023 18:48
@BrunoQuaresma BrunoQuaresma self-assigned this Jun 6, 2023
value,
id: "owner",
getSelectedOption: async () => {
const usersRes = await getUsers({ q: value, limit: 1 })

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.

We call getUsers here, and also on line 48. Curious why we can't just make one call to grab all users and then filter on the user we want for getSelectedOption.

@BrunoQuaresma BrunoQuaresma Jun 7, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good question, let's say we have a user called Wesley, Wesley is probably one of the last users to be queried in the database, so when we display the autocomplete options for the users, maybe Wesley is not shown because we only show the first 25 results. So making a separate query to get the selected value, in this case Wesley, is safer and we can be sure we are going to find it.

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.

Gotcha, thanks for explaining!


if (type === "workspace_build") {
label = "Workspace Build"
}

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.

This is fine! We could probably shorten with a map if we wanted to, but not a blocker.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I just went to what Copilot suggested to me 😆 since this is not complex code I think it is ok to let to refactor this if we need to extract it into a function to be reused anywhere else.

@Kira-Pilot Kira-Pilot left a comment

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.

Nice work!

@BrunoQuaresma BrunoQuaresma enabled auto-merge (squash) June 7, 2023 13:59
@BrunoQuaresma BrunoQuaresma merged commit d793564 into main Jun 7, 2023
@BrunoQuaresma BrunoQuaresma deleted the bq/add-filter-to-audit-logs branch June 7, 2023 14:11
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants