fix: fixed players search endpoint after Blizzard removal of Battle Tag on search #338
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Reviewer's GuideRefactors player search and profile parsing to work without BattleTag in Blizzard’s search API by matching on exact-case player names, supporting Blizzard hex IDs as player identifiers, and falling back to direct profile-page parsing when search results are ambiguous or empty, with tests updated accordingly. Sequence diagram for updated player career retrieval with search fallbacksequenceDiagram
actor User
participant APIRouter as APIRouter
participant BasePlayerParser as BasePlayerParser
participant SearchDataParser as SearchDataParser
participant BlizzardSearchAPI as BlizzardSearchAPI
participant CacheManager as CacheManager
participant PlayerCareerParser as PlayerCareerParser
participant BlizzardProfilePage as BlizzardProfilePage
User->>APIRouter: GET /players/{player_id}/career
APIRouter->>BasePlayerParser: parse(player_id)
activate BasePlayerParser
BasePlayerParser->>SearchDataParser: parse_data()
activate SearchDataParser
SearchDataParser->>BlizzardSearchAPI: GET /players/{player_name}/search
BlizzardSearchAPI-->>SearchDataParser: JSON search results
SearchDataParser->>SearchDataParser: extract player_name from player_id
SearchDataParser->>SearchDataParser: filter players by name and isPublic
SearchDataParser-->>BasePlayerParser: summary data or empty
deactivate SearchDataParser
alt summary data found
BasePlayerParser->>CacheManager: get_player_cache(player_id)
CacheManager-->>BasePlayerParser: cached data or None
alt cache hit and up_to_date
BasePlayerParser->>PlayerCareerParser: use cached data
PlayerCareerParser-->>BasePlayerParser: parsed career data
else cache miss or stale
BasePlayerParser->>BlizzardProfilePage: GET /players/{resolved_player_id}/profile
BlizzardProfilePage-->>BasePlayerParser: HTML profile page
BasePlayerParser->>PlayerCareerParser: parse(html, summary)
PlayerCareerParser-->>BasePlayerParser: parsed career data
BasePlayerParser->>CacheManager: update cache
end
BasePlayerParser-->>APIRouter: career response
APIRouter-->>User: 200 OK with career data
else no unique summary (zero or multiple matches)
BasePlayerParser->>BasePlayerParser: summary is empty
BasePlayerParser->>BlizzardProfilePage: GET /players/{input_player_id}/profile
BlizzardProfilePage-->>BasePlayerParser: HTML profile page
BasePlayerParser->>PlayerCareerParser: parse(html, no summary cache)
PlayerCareerParser-->>BasePlayerParser: parsed career data
BasePlayerParser-->>APIRouter: career response (no search cache)
APIRouter-->>User: 200 OK with career data
end
deactivate BasePlayerParser
Class diagram for updated player parsers and PlayerShort modelclassDiagram
class JSONParser {
+json_data dict
+get_blizzard_url(kwargs) str
+parse() None
}
class SearchDataParser {
+player_id str
+player_name str
+parse_data() dict
+get_blizzard_url(kwargs) str
}
class PlayerSearchParser {
+search_nickname str
+search_name str
+order_by str
+offset int
+limit int
+get_blizzard_url(kwargs) str
+parse_data() dict
+filter_players() list~dict~
+apply_transformations(players Iterable~dict~) list~dict~
}
class BasePlayerParser {
+player_id str
+player_data dict
+cache_manager CacheManager
+parse() None
}
class PlayerCareerParser {
+player_data dict
+parse(html str, summary dict) None
-__get_summary() dict
}
class PlayerShort {
+player_id str
+name str
+avatar HttpUrl
+namecard HttpUrl
+title str
+career_url HttpUrl
+blizzard_id str
}
class CacheManager {
+get_player_cache(player_id str) dict
+update_player_cache(player_id str, data dict) None
}
JSONParser <|-- SearchDataParser
JSONParser <|-- PlayerSearchParser
BasePlayerParser o--> SearchDataParser
BasePlayerParser o--> PlayerCareerParser
BasePlayerParser o--> CacheManager
PlayerSearchParser --> PlayerShort
Flow diagram for updated player search filtering and player_id selectionflowchart TD
A[Receive search request with name or BattleTag] --> B[Set search_nickname from input]
B --> C[Derive search_name by splitting on first dash]
C --> D[Call Blizzard search endpoint with search_name]
D --> E[Receive JSON list of players]
E --> F[Filter players where name equals search_name and isPublic is True]
F --> G{Any matching players?}
G -->|No| H[Return empty players list]
H --> I[Respond with empty results]
G -->|Yes| J{Number of matching players}
J -->|1| K[Single matching player]
K --> L{Input contains dash?}
L -->|Yes - looks like BattleTag| M[Set player_id to original search_nickname]
L -->|No - plain username| N[Set player_id to player.url from result]
J -->|More than 1| O[Multiple matching players]
O --> P[For each player set player_id to player.url]
M --> Q[Build PlayerShort objects]
N --> Q
P --> Q
Q --> R[Set name from player.name]
R --> S[Set avatar, namecard, title, blizzard_id, career_url]
S --> T[Return list of PlayerShort results]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- In
SearchDataParserandPlayerSearchParseryou both deriveplayer_name/search_nameby splitting on'-'; consider extracting this into a small shared helper to keep the parsing logic consistent and easier to update if Blizzard changes formats again. - In
SearchDataParser.parse_data, the warning message "not found in search results" is logged even when multiple players match; you might want to clarify the wording (e.g., "ambiguous search, N matching players") so logs distinguish between 0 and >1 results. - In
PlayerSearchParser.apply_transformations,player_idfalls back toplayer["url"]when there are multiple matches; it may be worth normalizing or documenting the expected format of this URL-derived ID to ensure it remains stable and compatible with the rest of the API.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `SearchDataParser` and `PlayerSearchParser` you both derive `player_name`/`search_name` by splitting on `'-'`; consider extracting this into a small shared helper to keep the parsing logic consistent and easier to update if Blizzard changes formats again.
- In `SearchDataParser.parse_data`, the warning message "not found in search results" is logged even when multiple players match; you might want to clarify the wording (e.g., "ambiguous search, N matching players") so logs distinguish between 0 and >1 results.
- In `PlayerSearchParser.apply_transformations`, `player_id` falls back to `player["url"]` when there are multiple matches; it may be worth normalizing or documenting the expected format of this URL-derived ID to ensure it remains stable and compatible with the rest of the API.
## Individual Comments
### Comment 1
<location> `tests/players/parsers/test_player_career_stats_parser.py:51` </location>
<code_context>
indirect=True,
)
@pytest.mark.asyncio
async def test_unknown_player_parser_blizzard_error(
player_career_stats_parser: PlayerCareerStatsParser,
+ player_html_data: str,
</code_context>
<issue_to_address>
**suggestion (testing):** Conflicting mock configuration (both side_effect and return_value) makes the HTTP mock harder to reason about.
`httpx.AsyncClient.get` is patched with both `side_effect` and `return_value`, but `side_effect` takes precedence so `return_value` is ignored. Please drop `return_value` and use only `side_effect` (as in the other tests) so the call sequence is clear. Also confirm that the asserted exception type still aligns with the new fallback-to-HTML behavior used in the other parser tests.
Suggested implementation:
```python
@pytest.mark.parametrize(
("player_career_stats_parser", "player_html_data"),
[(unknown_player_id, unknown_player_id)],
indirect=True,
)
@pytest.mark.asyncio
async def test_unknown_player_parser_blizzard_error(
player_career_stats_parser: PlayerCareerStatsParser,
player_html_data: str,
player_search_response_mock: Mock,
):
with (
pytest.raises(ParserBlizzardError),
patch(
"httpx.AsyncClient.get",
side_effect=[
# Players search call first
player_search_response_mock,
# Player profile page
],
) as mock_get,
):
```
1. If the second HTTP call in this test should now exercise the "fallback-to-HTML" behavior (as in other parser tests), ensure that the second element of the `side_effect` list is the appropriate mock/response (e.g. a mock Blizzard error response followed by an HTML response, or an exception) that leads to `ParserBlizzardError` being raised. Align it with whatever is used in the other tests that cover the new behavior.
2. Re-run the test suite and, if the failure mode has changed due to the new fallback behavior (e.g. a different custom exception is now raised for this scenario), update `pytest.raises(ParserBlizzardError)` to the correct exception type to keep expectations consistent with the parser’s actual behavior.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
TeKrop
added a commit
that referenced
this pull request
Dec 16, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Following the removal of Battle Tag from Blizzard players search URL, the following change was made : added support for Blizzard ID as possible input for player_id on player profile endpoints
Summary by Sourcery
Update player search and profile parsing to work without BattleTag-based search and support Blizzard ID fallbacks for player identification.
New Features:
Bug Fixes:
Enhancements:
Tests: