Group Gambit Improvements

[askvortsov1]

Needed for FriendsOfFlarum/user-directory#39

Changes proposed in this pull request:

• Add ID to fields searched in group gambit. This is needed to search for groups that have a space in their names. When we add in group slugs as per Add unique slug to groups #1991, this should be less necessary, but good to have for now.
• Use joins instead of looping in group gambit

Confirmed

• Frontend changes: tested on a local Flarum installation.
• Backend changes: tests are green (run composer test).

[clarkwinkelmann]

I'm globally ok with the approach, but I'm thinking it might be more appropriate to actually use is_numeric to make the difference between a name and ID.

So my suggestions would be, if the identifier is numeric, check IDs. Otherwise, do the check on the names.

That way if the name of a group is a number, those results will not be returned when a program uses the API to return the list of users for a specific other group that has that ID.

It will have the same issues as the current "get user" endpoint, but I think here it makes most sense to have ID return only the correct results, and "name" to be a convenience for the few searches that will be typed by hand https://github.com/flarum/core/blob/master/src/Api/Controller/ShowUserController.php#L51

Then there's just the question of the comma-separated list of identifiers. Either use a loop and do an is_numeric on each identifier before adding it to an orWhere, or use the first value to apply all of the OR to ids or names.

I hope that makes sense.

[askvortsov1]

I'm globally ok with the approach, but I'm thinking it might be more appropriate to actually use is_numeric to make the difference between a name and ID.

Is the goal to avoid groups named "123"? Personally I'd prefer if we could query with a slug instead of an ID, but there's currently no slug mechanism. Maybe I should make a group slug PR first, and then when that's merged we can use that here?

[clarkwinkelmann]

I think the slug thing needs more discussion before it happens, so I'd suggest making the ID the primary way to use the gambit for now, so that extensions like user-directory can start using it starting next beta even if we don't decide on anything for the slug before then.

[clarkwinkelmann]

Just reading the code, ok for me