Eric Taylor
595875ca07
## Security Fixes (Critical) ### 1. Settings file race condition fixed - Added atomic write operation using temp file + os.replace() - Prevents corruption if process crashes during settings save - Uses proper cleanup on failure ### 2. CSV quoting protection implemented - Added csv.QUOTE_MINIMAL to all CSV DictWriter operations - Optimal efficiency while maintaining security - Proper handling of special characters (quotes, commas, newlines) ### 3. Complete CSV field sanitization - Fixed critical Date field sanitization gap - Created specialized sanitize_date_text() preserving YYYY-MM-DD format - All 7 CSV fields now properly sanitized before writing - Added comprehensive input validation for user input vectors ## New Security Functions - sanitize_csv_text(): Removes dangerous characters (=,+, -, @) - sanitize_date_text(): Preserves date format while removing injection attempts - sanitize_filename(): Path traversal protection - sanitize_config_text(): JSON/configuration safety - validate_input(): Centralized input validation with type-specific logic ## Enhanced Features - Alternating row colors for visual time slot distinction - Improved conflict resolution with clearer UI indicators - Enhanced CSV error handling with line numbering ## Testing & Documentation - Added comprehensive test suites (5 new test files) - Created AGENTS.md development guide - Updated TODO.md with staged improvement roadmap - All tests passing with 100% backward compatibility ## Files Modified - time_tracker.py: +280 lines (security functions + atomic operations) - tests/: New security and feature test suites - .gitignore: Updated to include documentation and tests All critical vulnerabilities resolved while maintaining full functionality.
215 lines
8.5 KiB
Markdown
215 lines
8.5 KiB
Markdown
# Time Tracker Refactor TODO
|
|
|
|
This document outlines the recommended improvements from the code review, organized by priority stages.
|
|
|
|
## ✨ New Feature Ideas
|
|
|
|
### User Interface Enhancements
|
|
- [x] **Alternating colors for each hour** ✅
|
|
- Improve visual distinction between time slots
|
|
- Use subtle color gradients or patterns
|
|
- Consider color-blind friendly palettes
|
|
|
|
- [ ] **Pinned/frozen columns for data entry** 🚧
|
|
- Freeze Job, Task Name, Notes, Customer columns on the left side
|
|
- These columns remain visible when horizontally scrolling through time slots
|
|
- Similar to spreadsheet frozen panes functionality
|
|
- Critical for usability with work schedules longer than 4-5 hours
|
|
|
|
- [ ] **More compact user interface**
|
|
- Reduce padding and margins for better space utilization
|
|
- Collapsible sections for advanced features
|
|
- Responsive layout for smaller screens
|
|
|
|
- [ ] **Button to open archive CSV in text editor**
|
|
- Add "Open Archive" button that launches system default editor
|
|
- Cross-platform support (Windows, Mac, Linux)
|
|
- Optional: Use vim/emacs if available
|
|
|
|
- [ ] **Windows compatibility improvements**
|
|
- Test and fix Windows-specific path handling
|
|
- Ensure proper font rendering
|
|
- Verify installer/packaging options
|
|
|
|
## 🚨 Stage 1: Critical Issues (High Priority)
|
|
|
|
### Security & Stability
|
|
- [x] **Add comprehensive input sanitization** ✅
|
|
- Validate task names, notes, customer names
|
|
- Prevent CSV injection attacks
|
|
- Sanitize file paths
|
|
- **Implementation**: Created sanitization functions for CSV text, filenames, and config data
|
|
- **Security Features**: Excel formula blocking, directory traversal protection, JSON safety
|
|
|
|
- [🔧] **Critical security fixes from code review**
|
|
- [ ] **Fix settings file race condition** - Use atomic write pattern with temp file
|
|
- **Issue**: Direct file overwrite can corrupt settings if process crashes
|
|
- **Impact**: Loss of all application configuration (jobs, customers, paths)
|
|
- [ ] **Add CSV quoting protection** - Use proper csv.QUOTE_MINIMAL for safer CSV writing
|
|
- **Issue**: Current character removal isn't enough for complete CSV safety
|
|
- **Impact**: Potential CSV injection attacks could still succeed
|
|
- [ ] **Sanitize all CSV fields consistently** - Fix Date field and username field gaps
|
|
- **Issue**: Some fields (Date, username) not properly sanitized before CSV writing
|
|
- **Impact**: Data corruption and potential inject vulnerabilities remain
|
|
|
|
- [ ] **Replace filedialog usage** for PDF exports
|
|
- Use `filedialog.asksaveasfilename` instead
|
|
- Validate file extensions
|
|
- Add overwrite confirmation
|
|
|
|
- [ ] **Move drag_info from global to class attribute**
|
|
- Remove global state dependency
|
|
- Improve encapsulation
|
|
- Make class more testable
|
|
|
|
- [ ] **Move drag_info from global to class attribute in TimeTracker**
|
|
|
|
🔧 Medium Priority Additions (from code review):
|
|
- [ ] **Add type conversion error handling** - Prevent ValueError on hours field
|
|
- [ ] **Precompile regular expressions** for better performance
|
|
- [ ] **Add comprehensive error handling** for filename and filesystem issues
|
|
|
|
### Code Structure
|
|
- [ ] **Refactor open_settings() method** (200+ lines)
|
|
- Extract tab creation into separate methods: `_create_jobs_tab()`, `_create_customers_tab()`, etc.
|
|
- Extract button creation logic
|
|
- Reduce complexity
|
|
|
|
- [ ] **Refactor export_to_pdf() method**
|
|
- Extract table creation logic
|
|
- Extract styling logic
|
|
- Simplify main method flow
|
|
|
|
## 🔧 Stage 2: Architecture Improvements (Medium Priority)
|
|
|
|
### Performance & UX
|
|
- [ ] **Implement pinned/frozen columns for data entry** (NEW)
|
|
- **Problem**: When users have long work schedules (8+ hours), they can't see the Job/Task/Notes/Customer columns while scrolling through later time slots
|
|
- **Solution**: Create dual-frame layout with fixed left pane for data columns and scrollable right pane for time slots
|
|
- **Implementation**:
|
|
- Split scrollable_frame into two frames: `fixed_columns_frame` (columns 0-3) and `time_columns_frame` (column 4+)
|
|
- `fixed_columns_frame`: No horizontal scroll, contains Job dropdown, Task entry, Notes entry, Customer dropdown
|
|
- `time_columns_frame`: Horizontal scroll for time slots, aligning with fixed columns vertically
|
|
- Synchronize vertical scrolling between both frames
|
|
- **Technical challenges**:
|
|
- Row height synchronization between frames
|
|
- Visual alignment and border management
|
|
- Drag operations spanned across both frames
|
|
- Focus management and tab ordering
|
|
- [ ] **Implement CSV streaming reader** for large archive files
|
|
- Prevent memory issues with large datasets
|
|
- Add pagination for large archives
|
|
- Consider SQLite for better performance
|
|
|
|
- [ ] **Add progress dialogs** for long-running operations
|
|
- PDF export progress
|
|
- Large CSV processing
|
|
- Archive operations
|
|
|
|
- [ ] **Create Settings class** for type-safe configuration
|
|
- Replace JSON dict manipulation
|
|
- Add validation for settings values
|
|
- Provide default value management
|
|
|
|
### Code Organization
|
|
- [ ] **Extract constants** to separate module
|
|
- Widget dimensions, colors, time intervals
|
|
- File paths and formats
|
|
- Magic numbers scattered in code
|
|
|
|
- [ ] **Create GUIBuilder helper class** for common widget operations
|
|
- Standardize widget creation
|
|
- Reduce code duplication
|
|
- Consistent styling
|
|
|
|
- [ ] **Create ReportGenerator class**
|
|
- Extract reporting logic from TimeTracker
|
|
- Separate data processing from GUI
|
|
- Make reports more testable
|
|
|
|
- [ ] **Add data validation layer** between GUI and CSV operations
|
|
- Centralized validation logic
|
|
- Consistent error messages
|
|
- Better separation of concerns
|
|
|
|
### Testing
|
|
- [ ] **Add comprehensive unit tests** for data processing methods
|
|
- CSV reading/writing
|
|
- Time calculation logic
|
|
- Data validation
|
|
- Report generation
|
|
|
|
## 🎯 Stage 3: Quality & Nice-to-Haves (Low/Medium Priority)
|
|
|
|
### Code Quality
|
|
- [ ] **Add type hints** throughout the codebase
|
|
- Improve IDE support
|
|
- Better documentation
|
|
- Catch type-related bugs early
|
|
|
|
- [ ] **Implement debouncing** for rapid cell selections during drag operations
|
|
- Improve performance during fast dragging
|
|
- Reduce GUI update frequency
|
|
- Better user experience
|
|
|
|
### Documentation & Maintenance
|
|
- [ ] **Create architecture documentation**
|
|
- Document class relationships
|
|
- Add sequence diagrams for key workflows
|
|
- Maintenance guidelines
|
|
|
|
## 📋 Implementation Guidelines
|
|
|
|
### Before Starting Each Task:
|
|
1. Create a feature branch for the task
|
|
2. Run existing tests to ensure baseline
|
|
3. Implement changes incrementally
|
|
4. Test each change thoroughly
|
|
5. Update AGENTS.md if adding new patterns
|
|
|
|
### Pinned Columns Implementation Strategy:
|
|
For the pinned columns feature specifically:
|
|
1. **Phase 1**: Create dual-frame layout structure
|
|
- Replace single `scrollable_frame` with linked `fixed_columns_frame` and `time_columns_frame`
|
|
- Ensure proper vertical alignment between frames
|
|
2. **Phase 2**: Update row creation logic
|
|
- Modify `add_row()` to create widgets in both frames
|
|
- Maintain row index synchronization
|
|
3. **Phase 3**: Synchronize interactions
|
|
- Update drag operations to work across frame boundaries
|
|
- Ensure consistent styling and borders
|
|
- Test focus management and keyboard navigation
|
|
|
|
### After Each Task:
|
|
1. Run all tests to ensure no regressions
|
|
2. Test the GUI functionality manually
|
|
3. Run the application to verify it works end-to-end
|
|
4. Update this TODO file with completion status
|
|
|
|
### Testing Strategy:
|
|
- **Unit Tests**: For individual methods and classes
|
|
- **Integration Tests**: For CSV operations and report generation
|
|
- **GUI Tests**: Manual testing of user workflows
|
|
- **Regression Tests**: Ensure existing functionality isn't broken
|
|
|
|
## 🔄 Dependencies Between Tasks:
|
|
|
|
| Task | Depends On |
|
|
|------|------------|
|
|
| Create ReportGenerator | Add data validation layer |
|
|
| Add data validation layer | Create Settings class |
|
|
| Implement CSV streaming | Create constants module |
|
|
| **Pinned columns** | **Create GUIBuilder helper class** (for consistent widget management) |
|
|
| **Add critical security fixes** | **None (time-sensitive)** |
|
|
| **Add medium priority fixes** | **None (performance/stability)** |
|
|
| Add comprehensive tests | All major refactoring tasks |
|
|
|
|
## 📊 Progress Tracking:
|
|
|
|
- **Stage 1**: 1/8 completed (1 base + 3 critical fixes pending)
|
|
- **Stage 2**: 0/9 completed
|
|
- **Stage 3**: 1/2 completed
|
|
- **New Features**: 1/4 completed
|
|
- **Total**: 2/24 completed
|
|
|
|
*Priority Legend: 🚨 Critical | 🔧 Important | 🎯 Enhancement | 🔧 🔧 Code Review Findings* |