-
Notifications
You must be signed in to change notification settings - Fork 1
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add ServiceFilter query #130
Conversation
internal/app/user.go
Outdated
return userNames, nil | ||
} | ||
|
||
func (h *HeurekaApp) ListUniqueUserID(filter *entity.UserFilter, options *entity.ListOptions) ([]string, error) { |
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.
Maybe use plural? ListUniqueUserIDs
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.
@drochow please also review. I added a comment, but overall it looks good to me
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.
Good Job,
The code looks clean and neat BUT:
We are missing all SQL queries and Grouping.
Why should we call "ListServiceNames" and get the same service name returned multiple times? Then, I could call the normal List function.
After thinking about it and seeing the Implementation, I also think that from the "Frontend" perspective, it might make sense not to have a dedicated endpoint for that but rather a ListOption to only return unique results by, e.g., ServiceName, like a grouping parameter. I'd still separate the logic in the Backend as done here, but on the API, I really doubt an additional endpoint does make sense.
@MR2011
WDYT?
After discussing with @MR2011 I've got it. Lets keep it like it is for the Frontend devs 👍 |
No description provided.