diff --git a/api/client.py b/api/client.py index 14ea4a7..03698e9 100644 --- a/api/client.py +++ b/api/client.py @@ -310,43 +310,50 @@ class APIClient: raise APIException(f"PUT failed: {e}") async def patch( - self, - endpoint: str, + self, + endpoint: str, data: Optional[Dict[str, Any]] = None, object_id: Optional[int] = None, api_version: int = 3, - timeout: Optional[int] = None + timeout: Optional[int] = None, + use_query_params: bool = False ) -> Optional[Dict[str, Any]]: """ Make PATCH request to API. - + Args: endpoint: API endpoint data: Request payload (optional for some PATCH operations) object_id: Optional object ID api_version: API version (default: 3) timeout: Request timeout override - + use_query_params: If True, send data as query parameters instead of JSON body (default: False) + Returns: JSON response data - + Raises: APIException: For HTTP errors or network issues """ 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() - + 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 - - # Use json=data if data is provided, otherwise send empty body + + # Use json=data if data is provided and not using query params kwargs = {} - if data is not None: + if data is not None and not use_query_params: kwargs['json'] = data - + async with self._session.patch(url, timeout=request_timeout, **kwargs) as response: if response.status == 401: logger.error(f"Authentication failed for PATCH: {url}") @@ -361,11 +368,11 @@ class APIClient: error_text = await response.text() logger.error(f"PATCH error {response.status}: {url} - {error_text}") raise APIException(f"PATCH request failed with status {response.status}: {error_text}") - + result = await response.json() logger.debug(f"PATCH Response: {str(result)[:1200]}{'...' if len(str(result)) > 1200 else ''}") return result - + except aiohttp.ClientError as e: logger.error(f"HTTP client error for PATCH {url}: {e}") raise APIException(f"Network error: {e}") diff --git a/commands/profile/README.md b/commands/profile/README.md index 6bbe070..918711c 100644 --- a/commands/profile/README.md +++ b/commands/profile/README.md @@ -315,9 +315,12 @@ updated_player = await player_service.update_player(player_id, update_data) **Endpoints Used:** - `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 +**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 ### Test Coverage diff --git a/commands/profile/images.py b/commands/profile/images.py index c54f2bf..6361612 100644 --- a/commands/profile/images.py +++ b/commands/profile/images.py @@ -189,27 +189,12 @@ async def player_name_autocomplete( return [] try: - from utils.autocomplete import player_autocomplete_with_team_priority - return await player_autocomplete_with_team_priority(interaction, current) + # Use the shared autocomplete utility with team prioritization + from utils.autocomplete import player_autocomplete + return await player_autocomplete(interaction, current) except Exception: - # Fallback to basic autocomplete - try: - 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 [] + # Return empty list on error to avoid breaking autocomplete + return [] # Main Command Cog diff --git a/services/README.md b/services/README.md index 597196f..cfe5492 100644 --- a/services/README.md +++ b/services/README.md @@ -27,9 +27,28 @@ class BaseService(Generic[T]): 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 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 ``` +**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 ### Core Entity Services diff --git a/services/base_service.py b/services/base_service.py index 130f88c..e1677b3 100644 --- a/services/base_service.py +++ b/services/base_service.py @@ -307,6 +307,41 @@ class BaseService(Generic[T]): 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: """ Delete object by ID. diff --git a/services/player_service.py b/services/player_service.py index b71953c..7a94d1c 100644 --- a/services/player_service.py +++ b/services/player_service.py @@ -281,16 +281,20 @@ class PlayerService(BaseService[Player]): async def update_player(self, player_id: int, updates: dict) -> Optional[Player]: """ Update player information. - + Args: player_id: Player ID to update updates: Dictionary of fields to update - + Returns: 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: - return await self.update(player_id, updates) + return await self.patch(player_id, updates, use_query_params=True) except Exception as e: logger.error(f"Failed to update player {player_id}: {e}") return None