WIP: Comment waveform integration with todos and known issues
This commit is contained in:
86
CHANGES_AND_TODOS.md
Normal file
86
CHANGES_AND_TODOS.md
Normal file
@@ -0,0 +1,86 @@
|
|||||||
|
# Comment Waveform Integration - Changes and Todos
|
||||||
|
|
||||||
|
## Completed Changes
|
||||||
|
|
||||||
|
### 1. Database Schema Changes
|
||||||
|
- **Added timestamp column**: Added `timestamp` field (FLOAT, nullable) to `song_comments` table
|
||||||
|
- **Migration**: Updated `0004_rehearsal_sessions.py` migration to include timestamp column
|
||||||
|
- **Model**: Updated `SongComment` SQLAlchemy model in `api/src/rehearsalhub/db/models.py`
|
||||||
|
|
||||||
|
### 2. API Changes
|
||||||
|
- **Schema**: Updated `SongCommentRead` and `SongCommentCreate` schemas to include timestamp
|
||||||
|
- **Endpoint**: Modified comment creation endpoint to accept and store timestamp
|
||||||
|
- **Health Check**: Fixed API health check in docker-compose.yml to use Python instead of curl
|
||||||
|
|
||||||
|
### 3. Frontend Changes
|
||||||
|
- **Waveform Hook**: Added `addMarker` and `clearMarkers` functions to `useWaveform.ts`
|
||||||
|
- **Song Page**: Updated `SongPage.tsx` to display comment markers on waveform
|
||||||
|
- **Error Handling**: Added validation for finite time values in `seekTo` function
|
||||||
|
- **Null Safety**: Added checks for null/undefined timestamps
|
||||||
|
|
||||||
|
### 4. Infrastructure
|
||||||
|
- **Docker**: Fixed health check command to work in container environment
|
||||||
|
- **Build**: Successfully built and deployed updated frontend
|
||||||
|
|
||||||
|
## Known Issues
|
||||||
|
|
||||||
|
### Database Migration Not Applied
|
||||||
|
- **Error**: `column "timestamp" of relation "song_comments" does not exist`
|
||||||
|
- **Cause**: The migration in `0004_rehearsal_sessions.py` wasn't run on the existing database
|
||||||
|
- **Impact**: Attempting to create new comments with timestamps will fail
|
||||||
|
|
||||||
|
## Todos
|
||||||
|
|
||||||
|
### Critical (Blockers)
|
||||||
|
- [ ] Apply database migration to add timestamp column to song_comments table
|
||||||
|
- [ ] Verify migration runs successfully on fresh database
|
||||||
|
- [ ] Test comment creation with timestamps after migration
|
||||||
|
|
||||||
|
### High Priority
|
||||||
|
- [ ] Update frontend to send timestamp when creating comments
|
||||||
|
- [ ] Add user avatar support for comment markers
|
||||||
|
- [ ] Improve marker styling and positioning
|
||||||
|
|
||||||
|
### Medium Priority
|
||||||
|
- [ ] Add timestamp editing functionality
|
||||||
|
- [ ] Implement comment marker tooltips
|
||||||
|
- [ ] Add keyboard shortcuts for comment timestamping
|
||||||
|
|
||||||
|
### Low Priority
|
||||||
|
- [ ] Add documentation for the new features
|
||||||
|
- [ ] Create user guide for comment waveform integration
|
||||||
|
- [ ] Add tests for new functionality
|
||||||
|
|
||||||
|
## Migration Notes
|
||||||
|
|
||||||
|
The database migration needs to be applied manually since it wasn't picked up automatically. Steps to apply:
|
||||||
|
|
||||||
|
1. **For existing databases**: Run the migration SQL manually:
|
||||||
|
```sql
|
||||||
|
ALTER TABLE song_comments ADD COLUMN timestamp FLOAT;
|
||||||
|
```
|
||||||
|
|
||||||
|
2. **For new deployments**: The migration should run automatically as part of the startup process.
|
||||||
|
|
||||||
|
3. **Verification**: After migration, test comment creation with timestamps.
|
||||||
|
|
||||||
|
## Testing Instructions
|
||||||
|
|
||||||
|
After applying the migration:
|
||||||
|
|
||||||
|
1. Create a new comment with a timestamp
|
||||||
|
2. Verify the comment appears in the list with timestamp button
|
||||||
|
3. Click the timestamp button to seek to that position
|
||||||
|
4. Verify the comment marker appears on the waveform
|
||||||
|
5. Click the marker to scroll to the comment
|
||||||
|
6. Test with older comments (without timestamps) to ensure backward compatibility
|
||||||
|
|
||||||
|
## Files Modified
|
||||||
|
|
||||||
|
- `docker-compose.yml` - Health check fix
|
||||||
|
- `api/alembic/versions/0004_rehearsal_sessions.py` - Added timestamp migration
|
||||||
|
- `api/src/rehearsalhub/db/models.py` - Added timestamp field
|
||||||
|
- `api/src/rehearsalhub/schemas/comment.py` - Updated schemas
|
||||||
|
- `api/src/rehearsalhub/routers/songs.py` - Updated comment creation
|
||||||
|
- `web/src/hooks/useWaveform.ts` - Added marker functions
|
||||||
|
- `web/src/pages/SongPage.tsx` - Added waveform integration
|
||||||
Reference in New Issue
Block a user