mirror of
https://github.com/DRYTRIX/TimeTracker.git
synced 2025-12-29 07:10:17 -06:00
feat: improve error handling, performance logging, and PWA install UI
- Add session state clearing (expunge_all) after rollbacks in custom field definition error handlers to prevent stale session state - Add graceful error handling for missing link_templates table with proper rollback and session cleanup, preventing app crashes when migrations haven't been run - Add detailed performance logging to TaskService.list_tasks method to track timing of each query step for performance monitoring - Improve PWA install prompt UI with better toast integration, dismiss button, and proper DOM manipulation using requestAnimationFrame - Bump version to 4.5.0
This commit is contained in:
@@ -61,9 +61,10 @@ class CustomFieldDefinition(db.Model):
|
||||
)
|
||||
except RuntimeError:
|
||||
pass # No application context
|
||||
# Rollback the failed transaction
|
||||
# Rollback the failed transaction and clear session state
|
||||
try:
|
||||
db.session.rollback()
|
||||
db.session.expunge_all() # Clear all objects from session
|
||||
except Exception:
|
||||
pass
|
||||
return []
|
||||
@@ -104,9 +105,10 @@ class CustomFieldDefinition(db.Model):
|
||||
)
|
||||
except RuntimeError:
|
||||
pass # No application context
|
||||
# Rollback the failed transaction
|
||||
# Rollback the failed transaction and clear session state
|
||||
try:
|
||||
db.session.rollback()
|
||||
db.session.expunge_all() # Clear all objects from session
|
||||
except Exception:
|
||||
pass
|
||||
return []
|
||||
@@ -147,9 +149,10 @@ class CustomFieldDefinition(db.Model):
|
||||
)
|
||||
except RuntimeError:
|
||||
pass # No application context
|
||||
# Rollback the failed transaction
|
||||
# Rollback the failed transaction and clear session state
|
||||
try:
|
||||
db.session.rollback()
|
||||
db.session.expunge_all() # Clear all objects from session
|
||||
except Exception:
|
||||
pass
|
||||
return None
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
from datetime import datetime
|
||||
from app import db
|
||||
from sqlalchemy.exc import ProgrammingError
|
||||
|
||||
|
||||
class LinkTemplate(db.Model):
|
||||
@@ -65,8 +66,47 @@ class LinkTemplate(db.Model):
|
||||
|
||||
@classmethod
|
||||
def get_active_templates(cls, field_key=None):
|
||||
"""Get active link templates, optionally filtered by field_key"""
|
||||
query = cls.query.filter_by(is_active=True)
|
||||
if field_key:
|
||||
query = query.filter_by(field_key=field_key)
|
||||
return query.order_by(cls.order, cls.name).all()
|
||||
"""Get active link templates, optionally filtered by field_key.
|
||||
|
||||
Returns empty list if table doesn't exist (migration not run yet).
|
||||
"""
|
||||
try:
|
||||
query = cls.query.filter_by(is_active=True)
|
||||
if field_key:
|
||||
query = query.filter_by(field_key=field_key)
|
||||
return query.order_by(cls.order, cls.name).all()
|
||||
except ProgrammingError as e:
|
||||
# Handle case where link_templates table doesn't exist (migration not run)
|
||||
if "does not exist" in str(e.orig) or "relation" in str(e.orig).lower():
|
||||
try:
|
||||
from flask import current_app
|
||||
if current_app:
|
||||
current_app.logger.warning(
|
||||
"link_templates table does not exist. Run migration: flask db upgrade"
|
||||
)
|
||||
except RuntimeError:
|
||||
pass # No application context
|
||||
# Rollback the failed transaction and clear session state
|
||||
try:
|
||||
db.session.rollback()
|
||||
db.session.expunge_all() # Clear all objects from session
|
||||
except Exception:
|
||||
pass
|
||||
return []
|
||||
raise
|
||||
except Exception:
|
||||
# For other database errors, return empty list to prevent breaking the app
|
||||
try:
|
||||
from flask import current_app
|
||||
if current_app:
|
||||
current_app.logger.warning(
|
||||
"Could not query link_templates. Returning empty list."
|
||||
)
|
||||
except RuntimeError:
|
||||
pass # No application context
|
||||
# Rollback the failed transaction
|
||||
try:
|
||||
db.session.rollback()
|
||||
except Exception:
|
||||
pass
|
||||
return []
|
||||
|
||||
@@ -189,14 +189,24 @@ class TaskService:
|
||||
Returns:
|
||||
dict with 'tasks', 'pagination', and 'total' keys
|
||||
"""
|
||||
import time
|
||||
import logging
|
||||
from sqlalchemy.orm import joinedload
|
||||
from app.utils.timezone import now_in_app_timezone
|
||||
|
||||
query = self.task_repo.query()
|
||||
logger = logging.getLogger(__name__)
|
||||
start_time = time.time()
|
||||
step_start = time.time()
|
||||
|
||||
query = self.task_repo.query()
|
||||
logger.debug(f"[TaskService.list_tasks] Step 1: Initial query creation took {(time.time() - step_start) * 1000:.2f}ms")
|
||||
|
||||
step_start = time.time()
|
||||
# Eagerly load relations to prevent N+1
|
||||
query = query.options(joinedload(Task.project), joinedload(Task.assigned_user), joinedload(Task.creator))
|
||||
logger.debug(f"[TaskService.list_tasks] Step 2: Eager loading setup took {(time.time() - step_start) * 1000:.2f}ms")
|
||||
|
||||
step_start = time.time()
|
||||
# Apply filters
|
||||
if status:
|
||||
query = query.filter(Task.status == status)
|
||||
@@ -222,20 +232,31 @@ class TaskService:
|
||||
# Permission filter - non-admins only see their tasks
|
||||
if not is_admin and user_id:
|
||||
query = query.filter(db.or_(Task.assigned_to == user_id, Task.created_by == user_id))
|
||||
logger.debug(f"[TaskService.list_tasks] Step 3: Applying filters took {(time.time() - step_start) * 1000:.2f}ms")
|
||||
|
||||
step_start = time.time()
|
||||
# Order by priority, due date, created date
|
||||
query = query.order_by(Task.priority.desc(), Task.due_date.asc(), Task.created_at.asc())
|
||||
logger.debug(f"[TaskService.list_tasks] Step 4: Ordering query took {(time.time() - step_start) * 1000:.2f}ms")
|
||||
|
||||
step_start = time.time()
|
||||
# Paginate (always use pagination for performance)
|
||||
pagination = query.paginate(page=page, per_page=per_page, error_out=False)
|
||||
logger.debug(f"[TaskService.list_tasks] Step 5: Pagination query execution took {(time.time() - step_start) * 1000:.2f}ms (total: {pagination.total} tasks, page: {page}, per_page: {per_page})")
|
||||
|
||||
step_start = time.time()
|
||||
# Pre-calculate total_hours for all tasks in a single query to avoid N+1
|
||||
# This prevents the template from triggering individual queries for each task
|
||||
tasks = pagination.items
|
||||
logger.debug(f"[TaskService.list_tasks] Step 6: Getting pagination items took {(time.time() - step_start) * 1000:.2f}ms ({len(tasks)} tasks)")
|
||||
|
||||
if tasks:
|
||||
from app.models import TimeEntry, KanbanColumn
|
||||
step_start = time.time()
|
||||
task_ids = [task.id for task in tasks]
|
||||
logger.debug(f"[TaskService.list_tasks] Step 7: Extracting task IDs took {(time.time() - step_start) * 1000:.2f}ms")
|
||||
|
||||
step_start = time.time()
|
||||
# Calculate total hours for all tasks in one query
|
||||
results = (
|
||||
db.session.query(
|
||||
@@ -250,13 +271,16 @@ class TaskService:
|
||||
.all()
|
||||
)
|
||||
total_hours_map = {task_id: total_seconds for task_id, total_seconds in results}
|
||||
logger.debug(f"[TaskService.list_tasks] Step 8: Calculating total hours query took {(time.time() - step_start) * 1000:.2f}ms ({len(results)} results)")
|
||||
|
||||
step_start = time.time()
|
||||
# Pre-load kanban columns to avoid N+1 queries in status_display property
|
||||
# Load global columns (project_id is None) since tasks don't have project-specific columns
|
||||
kanban_columns = KanbanColumn.get_active_columns(project_id=None)
|
||||
status_display_map = {}
|
||||
for col in kanban_columns:
|
||||
status_display_map[col.key] = col.label
|
||||
logger.debug(f"[TaskService.list_tasks] Step 9: Loading kanban columns took {(time.time() - step_start) * 1000:.2f}ms ({len(kanban_columns)} columns)")
|
||||
|
||||
# Fallback status map if no columns found
|
||||
fallback_status_map = {
|
||||
@@ -267,6 +291,7 @@ class TaskService:
|
||||
"cancelled": "Cancelled",
|
||||
}
|
||||
|
||||
step_start = time.time()
|
||||
# Cache the calculated values on task objects to avoid property queries
|
||||
for task in tasks:
|
||||
total_seconds = total_hours_map.get(task.id, 0) or 0
|
||||
@@ -277,5 +302,9 @@ class TaskService:
|
||||
task.status,
|
||||
fallback_status_map.get(task.status, task.status.replace("_", " ").title())
|
||||
)
|
||||
logger.debug(f"[TaskService.list_tasks] Step 10: Caching task properties took {(time.time() - step_start) * 1000:.2f}ms")
|
||||
|
||||
total_time = (time.time() - start_time) * 1000
|
||||
logger.info(f"[TaskService.list_tasks] Total time: {total_time:.2f}ms (tasks: {len(tasks) if tasks else 0}, page: {page}, per_page: {per_page})")
|
||||
|
||||
return {"tasks": tasks, "pagination": pagination, "total": pagination.total}
|
||||
|
||||
@@ -1494,36 +1494,52 @@
|
||||
|
||||
// Show install button in UI
|
||||
if (window.toastManager) {
|
||||
const toast = window.toastManager.info('Install TimeTracker as an app!', 0);
|
||||
const btn = document.createElement('button');
|
||||
btn.textContent = 'Install';
|
||||
btn.className = 'ml-2 px-3 py-1 bg-primary text-white rounded hover:bg-primary/90';
|
||||
btn.onclick = async () => {
|
||||
if (!deferredPrompt) return;
|
||||
deferredPrompt.prompt();
|
||||
const { outcome } = await deferredPrompt.userChoice;
|
||||
if (outcome === 'accepted') {
|
||||
window.toastManager.success('App installed successfully!');
|
||||
// Create a non-dismissible toast so we can add custom buttons
|
||||
const toastId = window.toastManager.show({
|
||||
message: 'Install TimeTracker as an app!',
|
||||
type: 'info',
|
||||
duration: 0,
|
||||
dismissible: false
|
||||
});
|
||||
// Get the toast element after it's added to DOM
|
||||
requestAnimationFrame(() => {
|
||||
const toastElement = document.querySelector(`[data-toast-id="${toastId}"]`);
|
||||
if (toastElement) {
|
||||
const btn = document.createElement('button');
|
||||
btn.textContent = 'Install';
|
||||
btn.className = 'ml-2 px-3 py-1 bg-primary text-white rounded hover:bg-primary/90';
|
||||
btn.onclick = async () => {
|
||||
if (!deferredPrompt) return;
|
||||
deferredPrompt.prompt();
|
||||
const { outcome } = await deferredPrompt.userChoice;
|
||||
if (outcome === 'accepted') {
|
||||
window.toastManager.success('App installed successfully!');
|
||||
}
|
||||
// Always mark as dismissed after user interaction
|
||||
localStorage.setItem('pwa-install-dismissed', 'true');
|
||||
deferredPrompt = null;
|
||||
window.toastManager.dismiss(toastId);
|
||||
};
|
||||
|
||||
// Add a dismiss button
|
||||
const dismissBtn = document.createElement('button');
|
||||
dismissBtn.textContent = '×';
|
||||
dismissBtn.className = 'ml-2 px-2 py-1 text-gray-600 dark:text-gray-300 hover:bg-gray-200 dark:hover:bg-gray-700 rounded';
|
||||
dismissBtn.title = 'Dismiss permanently';
|
||||
dismissBtn.onclick = () => {
|
||||
localStorage.setItem('pwa-install-dismissed', 'true');
|
||||
deferredPrompt = null;
|
||||
window.toastManager.dismiss(toastId);
|
||||
};
|
||||
|
||||
// Insert buttons in the toast content area
|
||||
const content = toastElement.querySelector('.toast-content');
|
||||
if (content) {
|
||||
content.appendChild(btn);
|
||||
content.appendChild(dismissBtn);
|
||||
}
|
||||
}
|
||||
// Always mark as dismissed after user interaction
|
||||
localStorage.setItem('pwa-install-dismissed', 'true');
|
||||
deferredPrompt = null;
|
||||
toast.remove();
|
||||
};
|
||||
|
||||
// Add a dismiss button
|
||||
const dismissBtn = document.createElement('button');
|
||||
dismissBtn.textContent = '×';
|
||||
dismissBtn.className = 'ml-2 px-2 py-1 text-white hover:bg-white/20 rounded';
|
||||
dismissBtn.title = 'Dismiss permanently';
|
||||
dismissBtn.onclick = () => {
|
||||
localStorage.setItem('pwa-install-dismissed', 'true');
|
||||
deferredPrompt = null;
|
||||
toast.remove();
|
||||
};
|
||||
|
||||
toast.appendChild(btn);
|
||||
toast.appendChild(dismissBtn);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user