Skip to content
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

MG-2287 - Improve Search For Users #2288

Merged
merged 33 commits into from
Jul 2, 2024

Conversation

nyagamunene
Copy link
Contributor

@nyagamunene nyagamunene commented Jun 13, 2024

What type of PR is this?

This is a feature because it improve search for Users.

What does this do?

This enables back-end support for filtering by name.

Which issue(s) does this PR fix/relate to?

Have you included tests for your changes?

Yes, I have included tests for my changes.

Did you document any new/modified feature?

No. I have not updated the documentation because it will be done in a different PR

Notes

@nyagamunene nyagamunene self-assigned this Jun 13, 2024
@nyagamunene nyagamunene force-pushed the MG-2287-ImproveSearch branch 2 times, most recently from 1b50d92 to 5e2f5dd Compare June 23, 2024 20:04
@nyagamunene nyagamunene changed the title MG-2287 - Improve Search For Users, Things, Channels and Groups MG-2287 - Improve Search For Users Jun 23, 2024
@nyagamunene nyagamunene marked this pull request as ready for review June 27, 2024 13:49
Copy link
Collaborator

@dborovcanin dborovcanin left a comment

Choose a reason for hiding this comment

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

Let's make the search case insensitive.

pkg/clients/postgres/clients.go Outdated Show resolved Hide resolved
pkg/clients/postgres/clients.go Outdated Show resolved Hide resolved
cli/users.go Outdated
{
Use: "search <query> <user_auth_token>",
Short: "Search users",
Long: "Search users by name, id or identity\n" +
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use username instead of identity here.

users/service_test.go Show resolved Hide resolved
@@ -321,6 +321,25 @@ func (sdk mgSDK) ListUserThings(userID string, pm PageMetadata, token string) (T
return tp, nil
}

func (sdk mgSDK) SearchUsers(pm PageMetadata, token string) (UsersPage, errors.SDKError) {
Copy link
Contributor

Choose a reason for hiding this comment

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

how does this method differe from the Users method. since both call the same repository method, retrieve basic info and both have the same function signature, it is redundant

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so. It is calling search endpoint which calls the searchBasicInfo method

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, but the methods RetrieveAllBasicInfo and SearchBasicInfo are functionally simillar why do they have to be distinct functions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a slight difference on how we construct the query. Also the RetrieveAllBasicInfo is supposed to be remove later from the conversation we had with @arvindh123. Initially I was to rename it to searchBasicInfo but it was agreed to keep it as it is and create the method seperately searchBasicInfo

Copy link
Collaborator

Choose a reason for hiding this comment

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

It is not the same, but we should RetrieveBasicInfo in ListClients in case the user is not an admin. @nyagamunene

Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
@dborovcanin dborovcanin merged commit 86d896d into absmach:main Jul 2, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

Improve search for Users, Things, Channels, and Groups Bug: New user to the platform sees all users
3 participants