Add membership ID support to player search - #116
Conversation
Co-authored-by: owens1127 <98496129+owens1127@users.noreply.github.com>
Barecheck - Code coverage reportTotal: 92.92%Your code coverage diff: 0.02% ▴ ✅ All code changes are covered |
There was a problem hiding this comment.
Pull request overview
This PR enhances the player search endpoint to support membership ID lookups in addition to the existing name-based search functionality. When a purely numeric query is detected, the system now performs a direct membership ID lookup in parallel with the name-based search, merging the results with the membership ID match appearing first.
Changes:
- Added parallel membership ID lookup when query is purely numeric, using existing
getPlayerfunction - Implemented deduplication logic to prevent duplicate results when membership ID matches appear in both searches
- Added test case validating membership ID search returns correct player as first result
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/services/search/player-search.ts | Implements parallel membership ID and name-based search with deduplication logic |
| src/routes/player/search.test.ts | Adds test case for membership ID query functionality |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let results = nameResults | ||
| if (membershipIdResult) { | ||
| const membershipIdBigInt = BigInt(trimmedQuery) | ||
| if (!results.some(r => r.membershipId === membershipIdBigInt)) { |
There was a problem hiding this comment.
The membership ID lookup does not respect the opts.membershipType filter. When a user specifies a membershipType filter (e.g., membershipType=2 for PlayStation), the name-based search correctly filters by platform, but the direct membership ID lookup via getPlayer() does not apply this filter. This could result in returning a player from a different platform than requested.
Consider filtering the membershipIdResult by membershipType before prepending it to results, similar to how the test "full bungie name wrong platform" expects zero results when the platform doesn't match.
| if (!results.some(r => r.membershipId === membershipIdBigInt)) { | |
| const membershipTypeMatches = | |
| !opts.membershipType || membershipIdResult.membershipType === opts.membershipType | |
| if (membershipTypeMatches && !results.some(r => r.membershipId === membershipIdBigInt)) { |
| expect(data.parsed.results.length).toBeGreaterThan(0) | ||
| expect(data.parsed.results[0].membershipId).toBe(BigInt("4611686018467831285")) | ||
| } | ||
| }) |
There was a problem hiding this comment.
The test for membership ID queries should include a test case that validates the membershipType filter is respected. Currently, there's no test verifying that when a membershipType filter is provided with a membership ID query, only players from that platform are returned (or the result is filtered out if the platform doesn't match).
Consider adding a test similar to "full bungie name wrong platform" that queries by membership ID with a mismatched membershipType to ensure proper filtering.
| }) | |
| }) | |
| test("membership id wrong platform", async () => { | |
| const data = await t({ | |
| query: "4611686018467831285", | |
| membershipType: 2 | |
| }) | |
| if (data.type === "ok") { | |
| expect(data.parsed.results).toHaveLength(0) | |
| } | |
| }) |
| results = [membershipIdResult, ...results] | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
When a membership ID result is prepended to the name-based results, the total results array can exceed the requested opts.count limit. For example, if opts.count is 10 and the name search returns 10 results, prepending the membership ID result would yield 11 results total.
Consider either slicing the results array to maintain the count limit, or documenting that membership ID matches can cause the result count to be count+1.
| if (results.length > opts.count) { | |
| results = results.slice(0, opts.count) | |
| } |
The player search endpoint only supported name-based lookup. This rolls the membership ID lookup logic (already present in the
/player/{membershipId}/basicendpoint) into the search backend.Changes
searchForPlayer: When the query is purely numeric, runs a directgetPlayer(membershipId)lookup in parallel with the existing LIKE search viaPromise.all. The membership ID result is prepended to the results if not already present (deduped bymembershipId). Errors from the ID lookup are caught so they can't degrade name-based results.search.test.ts: Added a test case for membership ID queries asserting the correct player is returned as the first result.Existing name-based search behavior is fully preserved.
Original prompt
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.