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

feature request: search user by IDs/usernames/phones/emails #2167

Closed
gao-sun opened this issue Oct 15, 2022 · 19 comments · Fixed by #2639
Closed

feature request: search user by IDs/usernames/phones/emails #2167

gao-sun opened this issue Oct 15, 2022 · 19 comments · Fixed by #2639
Assignees
Labels

Comments

@gao-sun
Copy link
Member

gao-sun commented Oct 15, 2022

What problem did you meet?

/api/users?search= only supports fuzzy search on multiple fields. Sometimes life needs to be more accurate, e.g., search a user by a specific ID, or a batch of usernames, etc.

Describe what you'd like Logto to have

Support something like /api/users?search=id:1,2,3&mode=exact to return the exact search result on a specific field.

This issue includes:

  • Search by multiple values with or logic
  • The result should exactly match the input
  • Support id, username, phone and email match
  • Integration test cases to cover all new features

To discuss (we can figure them it together):

  • The search pattern (search=id:1,2,3 or search=1|2|3&field=id or something else? we are not sure yet)
@RutsuKun
Copy link

maybe better something like:

/api/users?search[id]=1&search[id]=2&search[id]=3&search[username]=Test

then if I'm not mistaken, will be able to automatically convert this to an array of ids usernames.

what do you think?

@gao-sun
Copy link
Member Author

gao-sun commented Oct 24, 2022

maybe better something like:

/api/users?search[id]=1&search[id]=2&search[id]=3&search[username]=Test

then if I'm not mistaken, will be able to automatically convert this to an array of ids usernames.

what do you think?

looks great! i have no concern if this works.

@5war00p
Copy link
Contributor

5war00p commented Nov 1, 2022

Even though the URL length doesn't matter, I still feel like keeping it like this is simpler and still readable,
/users?ids=[1,2,3]&usernames=[a,b,c]&mode=exact

this makes maintaining code easier by directly doing JSON.parser from the URL params and passing it to the API.

@5war00p
Copy link
Contributor

5war00p commented Nov 1, 2022

And also if it's not assigned to anyone I can take this up.

@gao-sun
Copy link
Member Author

gao-sun commented Nov 3, 2022

@5war00p here you go. let's confirm the tech design before you code. one of our engineers will follow this issue

@gao-sun
Copy link
Member Author

gao-sun commented Nov 3, 2022

@wangsijie will follow this issue.

@logto-io logto-io deleted a comment from wangsijie Nov 4, 2022
@5war00p
Copy link
Contributor

5war00p commented Nov 5, 2022

/users?ids=[1,2,3]&usernames=[a,b,c]&mode=exact

@wangsijie will this be good to go?

@wangsijie
Copy link
Contributor

wangsijie commented Nov 6, 2022

@5war00p sorry for the late reply.

I prefer @RutsuKun's idea which can take advantage of the common HTML standard. But some changes are needed, I tried:

/api/users?id=1&id=2&id=3&username=foo

and the ctx.query would be this without any additional parser:

{
  "id": ["1", "2", "3"],
  "username": "foo"
}

What do you think about this? I was worried that the JSON way may not be popular and is a bit heavy because we only need an array.

@5war00p
Copy link
Contributor

5war00p commented Nov 6, 2022

Yes, this also makes sense, by using .getAll with the same key will give an array. No need to do any additional effort like JSON.parse and wrapping it under a try-catch block.

image

But the only thing is URL will have more redundancy, while sharing URL it looks more lengthy. (but this is not a problem as per functionality).

@wangsijie
Copy link
Contributor

The "redundancy" is OK? Only when the array have a large length. And even for that case, it will still work.

@5war00p
Copy link
Contributor

5war00p commented Nov 6, 2022

Cool, then we can make in that way. How the UX will be for this? means how users will give mutliple ids or usernames, will it be space separated?

@wangsijie
Copy link
Contributor

Let's start with the API part.

As for the UI, @fleuraly will take a look and post updates here.

@ihsanguldur
Copy link
Contributor

hello, is this issue in development phase? @wangsijie

@wangsijie
Copy link
Contributor

@5war00p is working on this, could you post some updates?

@5war00p
Copy link
Contributor

5war00p commented Nov 16, 2022

Yeah, I've started it.

But I got stuck with DB issues after updating my pnpm version.

I have to fix that and move forward.

@kentio
Copy link

kentio commented Dec 10, 2022

how is the progress?

@wangsijie
Copy link
Contributor

@5war00p any updates?

@gao-sun
Copy link
Member Author

gao-sun commented Dec 12, 2022

i think @5war00p was busy for this issue, no worries!
according to our bounty rules and the popular demand on this feature, we'll take back the claim and our team will be responsible for implementation. we'll release the feature with the next version.

@gao-sun
Copy link
Member Author

gao-sun commented Dec 14, 2022

we'll publish this feature with our December release. feel free to open a new issue if it doesn't work. cheers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

6 participants