Commit Graph

3 Commits

Author SHA1 Message Date
Cal Corum
e9dd318e5a perf: reuse persistent aiohttp.ClientSession in GiphyService (#26)
All checks were successful
Build Docker Image / build (pull_request) Successful in 1m5s
Both get_disappointment_gif and get_gif previously created a new
ClientSession per call. Replace with a lazily-initialised shared
session stored on the instance, eliminating per-call TCP handshake
and DNS overhead.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-03-05 01:32:52 -06:00
Cal Corum
f4be20afb3 fix: address 7 security issues across the codebase
- Remove hardcoded Giphy API key from config.py, load from env var (#19)
- URL-encode query parameters in APIClient._add_params (#20)
- URL-encode Giphy search phrases before building request URLs (#21)
- Replace internal exception details with generic messages to users (#22)
- Replace all bare except: with except Exception: (#23)
- Guard interaction.guild access in has_player_role (#24)
- Replace MD5 with SHA-256 for command change detection hash (#32)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-20 09:54:53 -06:00
Cal Corum
0c6f7c8ffe CLAUDE: Fix GroupCog interaction bug and GIF display issues
This commit addresses critical bugs in the injury command system and
establishes best practices for Discord command groups.

## Critical Fixes

### 1. GroupCog → app_commands.Group Migration
- **Problem**: `commands.GroupCog` has a duplicate interaction processing bug
  causing "404 Unknown interaction" errors when deferring responses
- **Root Cause**: GroupCog triggers command handler twice, consuming the
  interaction token before the second execution can respond
- **Solution**: Migrated InjuryCog to InjuryGroup using `app_commands.Group`
  pattern (same as ChartManageGroup and ChartCategoryGroup)
- **Result**: Reliable command execution, no more 404 errors

### 2. GiphyService GIF URL Fix
- **Problem**: Giphy service returned web page URLs (https://giphy.com/gifs/...)
  instead of direct image URLs, preventing Discord embed display
- **Root Cause**: Code accessed `data.url` instead of `data.images.original.url`
- **Solution**: Updated both `get_disappointment_gif()` and `get_gif()` methods
  to use correct API response path for embeddable GIF URLs
- **Result**: GIFs now display correctly in Discord embeds

## Documentation

### Command Groups Best Practices (commands/README.md)
Added comprehensive section documenting:
- **Critical Warning**: Never use `commands.GroupCog` - use `app_commands.Group`
- **Technical Explanation**: Why GroupCog fails (duplicate execution bug)
- **Migration Guide**: Step-by-step conversion from GroupCog to Group
- **Comparison Table**: Key differences between the two approaches
- **Working Examples**: References to ChartManageGroup, InjuryGroup patterns

## Architecture Changes

### Injury Commands (`commands/injuries/`)
- Converted from `commands.GroupCog` to `app_commands.Group`
- Registration via `bot.tree.add_command()` instead of `bot.add_cog()`
- Removed workarounds for GroupCog duplicate interaction issues
- Clean defer/response pattern with `@logged_command` decorator

### GiphyService (`services/giphy_service.py`)
- Centralized from `commands/soak/giphy_service.py`
- Now returns direct GIF image URLs for Discord embeds
- Maintains Trump GIF filtering (legacy behavior)
- Added gif_url to log output for debugging

### Configuration (`config.py`)
- Added `giphy_api_key` and `giphy_translate_url` settings
- Environment variable support via `GIPHY_API_KEY`
- Default values provided for out-of-box functionality

## Files Changed
- commands/injuries/: New InjuryGroup with app_commands.Group pattern
- services/giphy_service.py: Centralized service with GIF URL fix
- commands/soak/giphy_service.py: Backwards compatibility wrapper
- commands/README.md: Command groups best practices documentation
- config.py: Giphy configuration settings
- services/__init__.py: GiphyService exports

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-10-16 22:15:42 -05:00