feat(space): extend manager member search to username/email/phone

[an9xyz]

Summary

The admin member list endpoint GET /v1/manager/spaces/:space_id/members previously matched the keyword query param only against user.name and space_member.uid. This extends the keyword filter to also cover user.username, user.email and user.phone.

Motivation: SSO / email-login users often have an empty name, so admins could not locate them by display name. Allowing username/email/phone makes the member search usable for those accounts (consistent with the user-module list search behavior).

Changes

• db_manager: factor the keyword condition into a single OR LIKE clause shared by both the list and count queries, so the page rows and the total count always use the same filter (no drift). Reuses the existing wildcard escaping (_ / % / \ treated as literals via ESCAPE).
• tests: add TestManager_Members_KeywordSearch covering match-by name / username / email / phone / uid, list-vs-count parity, and wildcard escaping. The user fixture table is now rebuilt with DROP + CREATE (instead of CREATE TABLE IF NOT EXISTS) so a reused test database picks up the new username / phone columns — MySQL 8 has no ADD COLUMN IF NOT EXISTS.
• swagger: document the members-list endpoint (first path entry for the space module).

Behavior / compatibility

• Request and response shapes are unchanged; only the set of columns keyword matches against is widened. Fully backward compatible for existing clients.
• Pagination (page_index default 1, page_size default 20 / max 200) and admin auth are unchanged.

Test plan

• go vet ./modules/space/
• go test ./modules/space/ (full package green)
• TestManager_Members_KeywordSearch — verified each of name/username/email/phone/uid matches exactly the expected member; list and count share the filter; underscore is escaped (not a wildcard)
• Reproduced the "stale test DB with old user schema" case and confirmed TestMain now self-heals via DROP + CREATE

Tracking: #201

[lml2468]

[APPROVE]

Clean, well-scoped feature — single shared memberSearchWhere keeps list and count queries in sync, proper wildcard escaping, good test coverage.

✅ Highlights

• memberSearchWhere shared by both list and count eliminates the "filter drift → pagination mismatch" class of bugs. Smart design.
• Wildcard escaping via escapeLike + ESCAPE '\\' — actually more correct than the user module's queryUserListWithPageAndKeyword which does raw "%" + keyword + "%" without escaping. Nice.
• Test coverage is thorough: each search column tested individually, list-vs-count parity asserted, underscore-as-literal pinned.
• DROP + CREATE in TestMain is pragmatic — the MySQL 8 ADD COLUMN IF NOT EXISTS gap is real, and the comment explains the rationale clearly.

🔵 Suggestions (non-blocking)

1. No index on search columns (u.name, u.username, u.email, u.phone): the 5-column OR LIKE '%…%' guarantees a full scan on the user table. Acceptable for an admin-only endpoint with low query volume, but worth keeping in mind if the user table grows past ~100k rows. A composite fulltext index or a dedicated search column could help if this ever becomes a bottleneck — but that's a cross-module concern, not specific to this PR.

CI: Build ✅ | Lint ✅ | Vet ✅ | i18n ✅ | Test pending.

[Jerry-Xin]

This PR is in scope for octo-server and the implementation looks mergeable; I found no blocking correctness, security, or permission issues.

💬 Non-blocking

🟡 Warning: modules/space/swagger/api.yaml:67 references token security but the new Space Swagger document still has no securityDefinitions.token. Other module Swagger files define this explicitly, and standalone validation/tooling may treat this as an undefined security scheme. Add the standard apiKey header definition for token.

🔵 Suggestion: modules/space/api_test.go:61 recreates the test user.phone column as VARCHAR(20), while production migrations now end at VARCHAR(100). Matching production here would avoid future fixture surprises.

✅ Highlights

The shared memberSearchWhere helper keeps list/count filtering aligned in modules/space/db_manager.go:194 and modules/space/db_manager.go:214.

The new test covers name, username, email, phone, uid, list/count parity, and LIKE wildcard escaping.

I attempted go test ./modules/space/, but local MySQL was unavailable: dial tcp 127.0.0.1:3306: connect: connection refused.

[yujiawei]

Code Review — PR #200 (octo-server)

Verdict: APPROVED — no blocking issues. This is a focused, well-tested change that extends the admin member-search keyword filter from (name, uid) to (name, username, email, phone, uid). The implementation is sound; the notes below are non-blocking suggestions.

What I verified ✅

• No SQL injection. keyword is fully parameterized end-to-end. memberSearchWhere (db_manager.go:39-48) builds the clause from a hardcoded column list and binds %keyword% positionally via builder.Where(clause, args...). Placeholder count (5) matches arg count (5); the ESCAPE constant contains no ?. No string interpolation of user input.
• No cross-space leakage. queryMembersAdmin/countMembersAdmin pin Where("sm.space_id=?", spaceId) as a separate Where() call before adding the OR-block. gocraft/dbr wraps each Where condition in its own parentheses (buildCond in condition.go), so the SQL renders as WHERE (sm.space_id=?) AND (name LIKE ? OR ... OR sm.uid LIKE ?). The space scope is correctly AND-ed; there is no AND/OR precedence bug.
• LIKE wildcards escaped. escapeLike escapes \ % _ (backslash first, correct order) and pairs with an explicit ESCAPE '\\' clause, guarding against sql_mode=NO_BACKSLASH_ESCAPES. The wildcard escaped test (api_manager_test.go:315-319) is genuinely meaningful: it would fail if escaping regressed.
• No new PII in the response. managerMemberResp returns only uid/name/role/status/created_at/updated_at. email/phone/username are now searchable but are never serialized into the response body — only matched.
• List/count parity. Both queries share the identical JOIN, base WHERE, and memberSearchWhere clause under the same guard — no filter drift. user.uid is UNIQUE in both prod and the fixture, so the LEFT JOIN cannot fan out COUNT(*) above the list row count. Orphan members (no user row) are handled consistently (NULL LIKE never matches; sm.uid LIKE still does).
• Schema alignment. Production user (modules/user/sql/20191106000003 + later ALTERs) has username, email, phone. The test fixture's DROP + CREATE of user is safe: it runs once in TestMain (schema only), per-test isolation uses DELETE FROM (not DROP), there is no t.Parallel() in the package, and no FK references user.
• go vet ./modules/space/ passes.

Security notes for manual attention (non-blocking)

This PR is correctly flagged security-sensitive. Two observations that a human owner may want to acknowledge — both are pre-existing and not introduced by this diff, but this PR changes their blast radius:

1. Cross-space PII existence-oracle (P2, pre-existing model). The /v1/manager group is gated by requireAdmin → CheckLoginRole, which passes for any global admin/superAdmin role with no per-space ownership check. By widening the keyword filter to email/phone, any platform admin can now probe GET /v1/manager/spaces/<any-space>/members?keyword=<email-or-phone> to confirm whether a given email/phone belongs to a member of an arbitrary space. The authorization model is unchanged by this PR, but the PII surface it exposes is wider. Suggest confirming this is acceptable for the admin tier and that these searches are covered by audit logging.

2. No keyword min-length/format validation (nit). A 1-character keyword becomes LIKE '%x%' across PII columns, enabling broad partial sweeps. Low impact (an admin can already list all members of a space, and PII is not returned), but a minimum keyword length and/or audit logging would reduce the probe surface.

Minor / maintainability (non-blocking)

• Pagination has no unique tiebreaker (P2, pre-existing). queryMembersAdmin orders by sm.role DESC, sm.created_at ASC with LIMIT/OFFSET but no unique final sort key. created_at is second-granularity and members bulk-added in the same second share it; with equal role, MySQL's order for tied rows is undefined and can drift between pages, skipping/duplicating a boundary row. Untouched by this PR, but it sits in the changed function — consider appending .OrderAsc("sm.uid").
• Test fixture diverges from prod schema (nit). The fixture user table omits prod columns (short_no, status, role, etc.) and declares search columns nullable where prod is NOT NULL. Harmless for this PR (only the 5 search columns are touched), but a latent trap for future code paths that depend on the omitted columns.
• Implicit test invariant (nit). The list and count share filter subtest (expects 2 for example.com) silently relies on u-owner having no user row. An inline comment noting u-owner is intentionally left without a user row would prevent a future fixture change from breaking it.

Overall: clean, correct, and backward-compatible. Approving.