CLAUDE: Fix /set-image command to use query parameters for API updates
The /set-image command was failing to persist player image updates to the
database. Investigation revealed a mismatch between how the bot sent PATCH
data versus how the database API expected it.
Root Cause:
- Database API endpoint (/api/v3/players/{id}) expects PATCH data as URL
query parameters, not JSON body
- Bot was sending: PATCH /api/v3/players/12288 {"vanity_card": "url"}
- API expected: PATCH /api/v3/players/12288?vanity_card=url
Changes Made:
1. api/client.py:
- Added use_query_params parameter to patch() method
- When enabled, sends data as URL query parameters instead of JSON body
- Maintains backward compatibility (defaults to JSON body)
2. services/base_service.py:
- Added use_query_params parameter to patch() method
- Passes parameter through to API client
3. services/player_service.py:
- Updated update_player() to use use_query_params=True
- Added documentation note about query parameter requirement
4. commands/profile/images.py:
- Fixed autocomplete to use correct utility function
- Changed from non-existent player_autocomplete_with_team_priority
- Now uses player_autocomplete from utils/autocomplete.py
Documentation Updates:
5. commands/profile/README.md:
- Updated API Integration section
- Documented PATCH endpoint uses query parameters
- Added note about automatic handling in player_service
6. services/README.md:
- Added PATCH vs PUT operations documentation
- Documented use_query_params parameter
- Included usage examples for both modes
Testing:
- Verified /set-image command now successfully persists image URLs
- Confirmed API returns updated player with vanity_card populated
- Validated both fancy-card and headshot updates work correctly
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
parent
aa7aab3901
commit
bf374c1b98
@ -310,43 +310,50 @@ class APIClient:
|
|||||||
raise APIException(f"PUT failed: {e}")
|
raise APIException(f"PUT failed: {e}")
|
||||||
|
|
||||||
async def patch(
|
async def patch(
|
||||||
self,
|
self,
|
||||||
endpoint: str,
|
endpoint: str,
|
||||||
data: Optional[Dict[str, Any]] = None,
|
data: Optional[Dict[str, Any]] = None,
|
||||||
object_id: Optional[int] = None,
|
object_id: Optional[int] = None,
|
||||||
api_version: int = 3,
|
api_version: int = 3,
|
||||||
timeout: Optional[int] = None
|
timeout: Optional[int] = None,
|
||||||
|
use_query_params: bool = False
|
||||||
) -> Optional[Dict[str, Any]]:
|
) -> Optional[Dict[str, Any]]:
|
||||||
"""
|
"""
|
||||||
Make PATCH request to API.
|
Make PATCH request to API.
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
endpoint: API endpoint
|
endpoint: API endpoint
|
||||||
data: Request payload (optional for some PATCH operations)
|
data: Request payload (optional for some PATCH operations)
|
||||||
object_id: Optional object ID
|
object_id: Optional object ID
|
||||||
api_version: API version (default: 3)
|
api_version: API version (default: 3)
|
||||||
timeout: Request timeout override
|
timeout: Request timeout override
|
||||||
|
use_query_params: If True, send data as query parameters instead of JSON body (default: False)
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
JSON response data
|
JSON response data
|
||||||
|
|
||||||
Raises:
|
Raises:
|
||||||
APIException: For HTTP errors or network issues
|
APIException: For HTTP errors or network issues
|
||||||
"""
|
"""
|
||||||
url = self._build_url(endpoint, api_version, object_id)
|
url = self._build_url(endpoint, api_version, object_id)
|
||||||
|
|
||||||
|
# Add data as query parameters if requested
|
||||||
|
if use_query_params and data:
|
||||||
|
params = [(k, str(v)) for k, v in data.items()]
|
||||||
|
url = self._add_params(url, params)
|
||||||
|
|
||||||
await self._ensure_session()
|
await self._ensure_session()
|
||||||
|
|
||||||
try:
|
try:
|
||||||
logger.debug(f"PATCH: {endpoint} id: {object_id} data: {data}")
|
logger.debug(f"PATCH: {endpoint} id: {object_id} data: {data} use_query_params: {use_query_params}")
|
||||||
|
|
||||||
request_timeout = aiohttp.ClientTimeout(total=timeout) if timeout else None
|
request_timeout = aiohttp.ClientTimeout(total=timeout) if timeout else None
|
||||||
|
|
||||||
# Use json=data if data is provided, otherwise send empty body
|
# Use json=data if data is provided and not using query params
|
||||||
kwargs = {}
|
kwargs = {}
|
||||||
if data is not None:
|
if data is not None and not use_query_params:
|
||||||
kwargs['json'] = data
|
kwargs['json'] = data
|
||||||
|
|
||||||
async with self._session.patch(url, timeout=request_timeout, **kwargs) as response:
|
async with self._session.patch(url, timeout=request_timeout, **kwargs) as response:
|
||||||
if response.status == 401:
|
if response.status == 401:
|
||||||
logger.error(f"Authentication failed for PATCH: {url}")
|
logger.error(f"Authentication failed for PATCH: {url}")
|
||||||
@ -361,11 +368,11 @@ class APIClient:
|
|||||||
error_text = await response.text()
|
error_text = await response.text()
|
||||||
logger.error(f"PATCH error {response.status}: {url} - {error_text}")
|
logger.error(f"PATCH error {response.status}: {url} - {error_text}")
|
||||||
raise APIException(f"PATCH request failed with status {response.status}: {error_text}")
|
raise APIException(f"PATCH request failed with status {response.status}: {error_text}")
|
||||||
|
|
||||||
result = await response.json()
|
result = await response.json()
|
||||||
logger.debug(f"PATCH Response: {str(result)[:1200]}{'...' if len(str(result)) > 1200 else ''}")
|
logger.debug(f"PATCH Response: {str(result)[:1200]}{'...' if len(str(result)) > 1200 else ''}")
|
||||||
return result
|
return result
|
||||||
|
|
||||||
except aiohttp.ClientError as e:
|
except aiohttp.ClientError as e:
|
||||||
logger.error(f"HTTP client error for PATCH {url}: {e}")
|
logger.error(f"HTTP client error for PATCH {url}: {e}")
|
||||||
raise APIException(f"Network error: {e}")
|
raise APIException(f"Network error: {e}")
|
||||||
|
|||||||
@ -315,9 +315,12 @@ updated_player = await player_service.update_player(player_id, update_data)
|
|||||||
|
|
||||||
**Endpoints Used:**
|
**Endpoints Used:**
|
||||||
- `GET /api/v3/players?name={name}&season={season}` - Player search
|
- `GET /api/v3/players?name={name}&season={season}` - Player search
|
||||||
- `PUT /api/v3/players/{player_id}` - Update player data
|
- `PATCH /api/v3/players/{player_id}?vanity_card={url}` - Update player data
|
||||||
- `GET /api/v3/teams?owner_id={user_id}&season={season}` - User's teams
|
- `GET /api/v3/teams?owner_id={user_id}&season={season}` - User's teams
|
||||||
|
|
||||||
|
**Important Note:**
|
||||||
|
The player PATCH endpoint uses **query parameters** instead of JSON body for data updates. The `player_service.update_player()` method automatically handles this by setting `use_query_params=True` when calling the API client.
|
||||||
|
|
||||||
## Testing
|
## Testing
|
||||||
|
|
||||||
### Test Coverage
|
### Test Coverage
|
||||||
|
|||||||
@ -189,27 +189,12 @@ async def player_name_autocomplete(
|
|||||||
return []
|
return []
|
||||||
|
|
||||||
try:
|
try:
|
||||||
from utils.autocomplete import player_autocomplete_with_team_priority
|
# Use the shared autocomplete utility with team prioritization
|
||||||
return await player_autocomplete_with_team_priority(interaction, current)
|
from utils.autocomplete import player_autocomplete
|
||||||
|
return await player_autocomplete(interaction, current)
|
||||||
except Exception:
|
except Exception:
|
||||||
# Fallback to basic autocomplete
|
# Return empty list on error to avoid breaking autocomplete
|
||||||
try:
|
return []
|
||||||
players = await player_service.search_players(current, limit=25, season=SBA_CURRENT_SEASON)
|
|
||||||
|
|
||||||
choices = []
|
|
||||||
for player in players[:25]:
|
|
||||||
display_name = f"{player.name} ({player.primary_position})"
|
|
||||||
if hasattr(player, 'team') and player.team:
|
|
||||||
display_name += f" - {player.team.abbrev}"
|
|
||||||
|
|
||||||
choices.append(app_commands.Choice(
|
|
||||||
name=display_name,
|
|
||||||
value=player.name
|
|
||||||
))
|
|
||||||
|
|
||||||
return choices
|
|
||||||
except Exception:
|
|
||||||
return []
|
|
||||||
|
|
||||||
|
|
||||||
# Main Command Cog
|
# Main Command Cog
|
||||||
|
|||||||
@ -27,9 +27,28 @@ class BaseService(Generic[T]):
|
|||||||
async def get_all(self, params: Optional[List[tuple]] = None) -> Tuple[List[T], int]
|
async def get_all(self, params: Optional[List[tuple]] = None) -> Tuple[List[T], int]
|
||||||
async def create(self, model_data: Dict[str, Any]) -> Optional[T]
|
async def create(self, model_data: Dict[str, Any]) -> Optional[T]
|
||||||
async def update(self, object_id: int, model_data: Dict[str, Any]) -> Optional[T]
|
async def update(self, object_id: int, model_data: Dict[str, Any]) -> Optional[T]
|
||||||
|
async def patch(self, object_id: int, model_data: Dict[str, Any], use_query_params: bool = False) -> Optional[T]
|
||||||
async def delete(self, object_id: int) -> bool
|
async def delete(self, object_id: int) -> bool
|
||||||
```
|
```
|
||||||
|
|
||||||
|
**PATCH vs PUT Operations:**
|
||||||
|
- `update()` uses HTTP PUT for full resource replacement
|
||||||
|
- `patch()` uses HTTP PATCH for partial updates
|
||||||
|
- `use_query_params=True` sends data as URL query parameters instead of JSON body
|
||||||
|
|
||||||
|
**When to use `use_query_params=True`:**
|
||||||
|
Some API endpoints (notably the player PATCH endpoint) expect data as query parameters instead of JSON body. Example:
|
||||||
|
|
||||||
|
```python
|
||||||
|
# Standard PATCH with JSON body
|
||||||
|
await base_service.patch(object_id, {"field": "value"})
|
||||||
|
# → PATCH /api/v3/endpoint/{id} with JSON: {"field": "value"}
|
||||||
|
|
||||||
|
# PATCH with query parameters
|
||||||
|
await base_service.patch(object_id, {"field": "value"}, use_query_params=True)
|
||||||
|
# → PATCH /api/v3/endpoint/{id}?field=value
|
||||||
|
```
|
||||||
|
|
||||||
## Service Files
|
## Service Files
|
||||||
|
|
||||||
### Core Entity Services
|
### Core Entity Services
|
||||||
|
|||||||
@ -307,6 +307,41 @@ class BaseService(Generic[T]):
|
|||||||
|
|
||||||
return await self.update(model.id, model.to_dict(exclude_none=True))
|
return await self.update(model.id, model.to_dict(exclude_none=True))
|
||||||
|
|
||||||
|
async def patch(self, object_id: int, model_data: Dict[str, Any], use_query_params: bool = False) -> Optional[T]:
|
||||||
|
"""
|
||||||
|
Update existing object with HTTP PATCH.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
object_id: ID of object to update
|
||||||
|
model_data: Dictionary of fields to update
|
||||||
|
use_query_params: If True, send data as query parameters instead of JSON body
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
Updated model instance or None if not found
|
||||||
|
|
||||||
|
Raises:
|
||||||
|
APIException: For API errors
|
||||||
|
"""
|
||||||
|
try:
|
||||||
|
client = await self.get_client()
|
||||||
|
response = await client.patch(self.endpoint, model_data, object_id, use_query_params=use_query_params)
|
||||||
|
|
||||||
|
if not response:
|
||||||
|
logger.debug(f"{self.model_class.__name__} {object_id} not found for update")
|
||||||
|
return None
|
||||||
|
|
||||||
|
model = self.model_class.from_api_data(response)
|
||||||
|
logger.debug(f"Updated {self.model_class.__name__} {object_id}: {model}")
|
||||||
|
return model
|
||||||
|
|
||||||
|
except APIException:
|
||||||
|
logger.error(f"API error updating {self.model_class.__name__} {object_id}")
|
||||||
|
raise
|
||||||
|
except Exception as e:
|
||||||
|
logger.error(f"Error updating {self.model_class.__name__} {object_id}: {e}")
|
||||||
|
raise APIException(f"Failed to update {self.model_class.__name__}: {e}")
|
||||||
|
|
||||||
|
|
||||||
async def delete(self, object_id: int) -> bool:
|
async def delete(self, object_id: int) -> bool:
|
||||||
"""
|
"""
|
||||||
Delete object by ID.
|
Delete object by ID.
|
||||||
|
|||||||
@ -281,16 +281,20 @@ class PlayerService(BaseService[Player]):
|
|||||||
async def update_player(self, player_id: int, updates: dict) -> Optional[Player]:
|
async def update_player(self, player_id: int, updates: dict) -> Optional[Player]:
|
||||||
"""
|
"""
|
||||||
Update player information.
|
Update player information.
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
player_id: Player ID to update
|
player_id: Player ID to update
|
||||||
updates: Dictionary of fields to update
|
updates: Dictionary of fields to update
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
Updated player instance or None
|
Updated player instance or None
|
||||||
|
|
||||||
|
Note:
|
||||||
|
The player PATCH endpoint uses query parameters instead of JSON body,
|
||||||
|
so we pass use_query_params=True to the patch method.
|
||||||
"""
|
"""
|
||||||
try:
|
try:
|
||||||
return await self.update(player_id, updates)
|
return await self.patch(player_id, updates, use_query_params=True)
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.error(f"Failed to update player {player_id}: {e}")
|
logger.error(f"Failed to update player {player_id}: {e}")
|
||||||
return None
|
return None
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user