- Notifications
You must be signed in to change notification settings - Fork928
chore: allow organization name or uuid for audit log searching#13721
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I don't think we should remove the existing ability to query byorganization_id
.
@@ -177,13 +177,48 @@ func TestAuditLogs(t *testing.T) { | |||
// Using the organization selector allows the org admin to fetch audit logs | |||
alogs, err := orgAdmin.AuditLogs(ctx, codersdk.AuditLogsRequest{ | |||
SearchQuery: fmt.Sprintf("organization_id:%s", owner.OrganizationID.String()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
we should continue testing that searching byorganization_id
works
EDIT: undocumented feature
} else { | ||
// Organization could be a name | ||
organization, err := db.GetOrganizationByName(ctx, organizationArg) | ||
if err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
ah my bad, I fixated on line 33 but missed this check 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yea, I use the same query param oforganization
. Feels more straight forward
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Might be worth adding some sort of error if you useorganization_id
to suggest usingorganization
.
Note that this param is not documented or used anywhere yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Only problem is this might break existing queries, idk if any folks would have a reason to use this yet?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Note that this param is not documented or used anywhere yet.
Ah, if it's not documented then all bets are off.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yea, I added it 2 days ago, and intentionally am not documenting any of the multi-org stuff yet:#13663
6daf330
intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
What this does
Allows using organization name in the
organization
filter for audit log searching.