mirror of
https://github.com/DRYTRIX/TimeTracker.git
synced 2026-05-22 14:20:47 -05:00
Dries Peeters
d1c3471ba4
- Add ENHANCEMENT_PLAN_IMPLEMENTATION_STATUS.md - Add ENHANCEMENT_PLAN_PROGRESS_SUMMARY.md - Add COMMENT_ATTACHMENTS_IMPLEMENTATION.md - Add COMMENT_ATTACHMENTS_OPTIMIZATION.md - Add FINAL_IMPLEMENTATION_SUMMARY.md Documentation covers all enhancements, implementation status, remaining work, and optimization recommendations.
112 lines
3.1 KiB
Markdown
112 lines
3.1 KiB
Markdown
# Comment Attachments Performance Optimization
|
|
|
|
**Date:** 2025-01-27
|
|
**Status:** Recommended Enhancement
|
|
|
|
---
|
|
|
|
## Overview
|
|
|
|
Comment attachments are loaded using a `lazy="dynamic"` relationship, which means attachments are loaded on-demand when accessed. This can lead to N+1 query problems when displaying multiple comments with attachments.
|
|
|
|
---
|
|
|
|
## Current Implementation
|
|
|
|
### Relationship Definition
|
|
```python
|
|
# app/models/comment_attachment.py
|
|
comment = db.relationship("Comment", backref=db.backref("attachments", lazy="dynamic", cascade="all, delete-orphan"))
|
|
```
|
|
|
|
The `lazy="dynamic"` means:
|
|
- `comment.attachments` returns a query object, not a list
|
|
- Accessing `comment.attachments` triggers a database query
|
|
- Iterating over attachments in templates will work (SQLAlchemy auto-executes), but each comment triggers a separate query
|
|
|
|
---
|
|
|
|
## Performance Issue
|
|
|
|
When displaying a list of comments with attachments:
|
|
1. Load all comments (1 query)
|
|
2. For each comment, access `comment.attachments` (N queries, one per comment)
|
|
3. Total: 1 + N queries (N+1 problem)
|
|
|
|
---
|
|
|
|
## Recommended Solution
|
|
|
|
Use `selectinload()` to eager load attachments when querying comments:
|
|
|
|
### Task View Route (Already Updated)
|
|
```python
|
|
# app/routes/tasks.py - UPDATED
|
|
from sqlalchemy.orm import selectinload
|
|
|
|
all_comments = (
|
|
Comment.query.filter_by(task_id=task_id)
|
|
.options(
|
|
joinedload(Comment.author),
|
|
selectinload(Comment.replies).joinedload(Comment.author),
|
|
selectinload(Comment.attachments) # Added
|
|
)
|
|
.order_by(Comment.created_at.asc())
|
|
.all()
|
|
)
|
|
```
|
|
|
|
### Project Service (Needs Update)
|
|
If `ProjectService.get_project_view_data()` loads comments, it should also eager load attachments:
|
|
|
|
```python
|
|
# In ProjectService.get_project_view_data()
|
|
from sqlalchemy.orm import selectinload
|
|
|
|
comments = (
|
|
Comment.query.filter_by(project_id=project_id)
|
|
.options(
|
|
joinedload(Comment.author),
|
|
selectinload(Comment.replies).joinedload(Comment.author),
|
|
selectinload(Comment.attachments) # Add this
|
|
)
|
|
.order_by(Comment.created_at.asc())
|
|
.all()
|
|
)
|
|
```
|
|
|
|
---
|
|
|
|
## Benefits
|
|
|
|
1. **Performance**: Reduces N+1 queries to 2 queries (comments + attachments)
|
|
2. **Scalability**: Works efficiently with many comments
|
|
3. **Consistency**: Matches pattern used for replies and authors
|
|
|
|
---
|
|
|
|
## Implementation Status
|
|
|
|
- ✅ **Task View Route**: Updated to eager load attachments
|
|
- ⏳ **Project Service**: Needs review and update
|
|
- ⏳ **Quote Comments**: Needs review if quotes have comments
|
|
- ⏳ **API Endpoints**: May benefit from eager loading
|
|
|
|
---
|
|
|
|
## Testing
|
|
|
|
After implementing, verify:
|
|
1. No N+1 queries in database logs
|
|
2. Comments with attachments load correctly
|
|
3. Performance improvement with many comments
|
|
4. No breaking changes to existing functionality
|
|
|
|
---
|
|
|
|
## Notes
|
|
|
|
- `selectinload()` is preferred over `joinedload()` for one-to-many relationships (like attachments)
|
|
- `selectinload()` uses a separate SELECT IN query, which is more efficient than joins for collections
|
|
- The dynamic relationship still works for programmatic access, but templates benefit from eager loading
|