- Notifications
You must be signed in to change notification settings - Fork928
chore: add /groups endpoint to filter byorganization and/ormember#14260
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
organization and/ormember813a4da to190bd90Comparealwaysmeticulousbot commentedAug 13, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
✅ Meticulous spotted zero visual differences across 1412 screens tested:view results. Expected differences?Click here. Last updated for commitee3c8e4. This comment will update as new commits are pushed. |
| source | ||
| ) | ||
| SELECT | ||
| gen_random_uuid(), | ||
| group_name, | ||
| @organization_id, | ||
| @source | ||
| gen_random_uuid(), | ||
| group_name, | ||
| @organization_id, | ||
| @source | ||
| FROM | ||
| UNNEST(@group_names :: text[]) AS group_name | ||
| UNNEST(@group_names :: text[]) AS group_name |
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.
weird spacing here
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.
SQL spacing is so annoying. I wish there was some nice formatter that just worked.
| // UUIDorName will parse a string as a UUID, if it fails, it uses the "fetchByName" | ||
| // function to return a UUID based on the value as a string. | ||
| // This is useful when fetching something like an organization by ID or by name. | ||
| func (p *QueryParamParser) UUIDorName(vals url.Values, def uuid.UUID, queryParam string, fetchByName func(name string) (uuid.UUID, error)) uuid.UUID { |
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.
nit: having the query param 3rd here feels off to me personally, I'd expect the order values, query param, uuid, fetchByName.
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 see that, unfortunately the other functions all dodefault, queryParam so it felt odd to switch the order of those two.
I agree with you though, I think all the functions could be reordered, but don't want to do that refactor here.
7b09d98 intomainUh oh!
There was an error while loading.Please reload this page.
Closes#14249