From 7dcd58608ad2b53de4884c75e252327f7597df46 Mon Sep 17 00:00:00 2001 From: Dries Peeters Date: Thu, 22 Jan 2026 13:35:08 +0100 Subject: [PATCH] feat: Enhance TimeEntry audit logging with comprehensive tracking Add comprehensive audit logging for TimeEntry operations including: - Client/project context and creation timestamps - Full entity state before/after changes - User-provided reasons for deletions and modifications - Enhanced UI for entering reasons in delete/edit dialogs Database Changes: - Add migration 114: reason, entity_metadata, full_old_state, full_new_state columns - Use JSON column type for entity_metadata for better type handling Model Updates: - Extend AuditLog model with new fields and helper methods - Update log_change() to accept reason, metadata, and full states - Add get_entity_metadata(), get_full_old_state(), get_full_new_state() methods - Use JSON column for entity_metadata (returns dict/list directly) Service Layer: - Update TimeTrackingService to capture full TimeEntry state and metadata - Accept reason parameter in delete_entry() and update_entry() - Create comprehensive audit logs with all context API Routes: - Update api.py, api_v1.py, and timer.py routes to accept reason parameter - Refactor routes to use service layer for consistent audit logging - Add reason support to bulk delete operations UI Enhancements: - Add reason textarea to bulk delete confirmation dialog - Add reason textarea to time entry edit forms (admin and regular users) - Update JavaScript to handle reason submission Audit Log Display: - Show client/project information and creation timestamp in list view - Display full old/new states, reason, and metadata in detail view - Format JSON states for better readability Bug Fixes: - Fix duration_seconds reference in timer stop route - Improve error handling in timer operations with proper exception handling - Add dashboard cache invalidation after manual entry creation --- app/models/audit_log.py | 49 +++++ app/models/time_entry.py | 8 +- app/routes/api.py | 155 +++++----------- app/routes/api_v1.py | 12 +- app/routes/timer.py | 170 +++++++++++------- app/services/time_tracking_service.py | 102 ++++++++++- app/templates/audit_logs/list.html | 26 ++- app/templates/audit_logs/view.html | 76 +++++++- .../timer/time_entries_overview.html | 50 +++++- app/utils/audit.py | 97 ++++++++++ .../114_enhance_audit_logs_for_timeentry.py | 100 +++++++++++ templates/timer/edit_timer.html | 18 ++ 12 files changed, 670 insertions(+), 193 deletions(-) create mode 100644 migrations/versions/114_enhance_audit_logs_for_timeentry.py diff --git a/app/models/audit_log.py b/app/models/audit_log.py index dcf56d85..98aed83b 100644 --- a/app/models/audit_log.py +++ b/app/models/audit_log.py @@ -45,6 +45,18 @@ class AuditLog(db.Model): # Human-readable change description change_description = db.Column(db.Text, nullable=True) + # User-provided reason for change/deletion + reason = db.Column(db.Text, nullable=True) + + # Entity metadata (JSON-encoded for client_id, project_id, created_at, etc.) + entity_metadata = db.Column(db.JSON, nullable=True) # JSON metadata (dict/list) + + # Full entity state before change (JSON-encoded) + full_old_state = db.Column(db.Text, nullable=True) + + # Full entity state after change (JSON-encoded) + full_new_state = db.Column(db.Text, nullable=True) + # Additional context ip_address = db.Column(db.String(45), nullable=True) user_agent = db.Column(db.Text, nullable=True) @@ -79,6 +91,10 @@ class AuditLog(db.Model): new_value=None, entity_name=None, change_description=None, + reason=None, + entity_metadata=None, + full_old_state=None, + full_new_state=None, ip_address=None, user_agent=None, request_path=None, @@ -95,6 +111,10 @@ class AuditLog(db.Model): new_value: New value (will be JSON-encoded) entity_name: Cached name of the entity for display change_description: Human-readable description of the change + reason: User-provided reason for the change/deletion + entity_metadata: Metadata dict/list (client_id, project_id, created_at, etc.) - will be stored as JSON + full_old_state: JSON-encoded full entity state before change + full_new_state: JSON-encoded full entity state after change ip_address: IP address of the request user_agent: User agent string request_path: Path of the request that triggered the change @@ -102,6 +122,11 @@ class AuditLog(db.Model): # Encode values as JSON if they're not already strings old_val_str = cls._encode_value(old_value) new_val_str = cls._encode_value(new_value) + + # entity_metadata is stored as JSON type, so pass dict/list directly (not encoded) + # full_old_state and full_new_state are Text columns, so encode as JSON strings + full_old_str = cls._encode_value(full_old_state) if full_old_state else None + full_new_str = cls._encode_value(full_new_state) if full_new_state else None audit_log = cls( user_id=user_id, @@ -113,6 +138,10 @@ class AuditLog(db.Model): new_value=new_val_str, entity_name=entity_name, change_description=change_description, + reason=reason, + entity_metadata=entity_metadata, # Pass dict/list directly for JSON column + full_old_state=full_old_str, + full_new_state=full_new_str, ip_address=ip_address, user_agent=user_agent, request_path=request_path, @@ -178,6 +207,22 @@ class AuditLog(db.Model): """Get the decoded new value""" return self._decode_value(self.new_value) + def get_entity_metadata(self): + """Get the entity metadata (already a dict/list for JSON column)""" + # For JSON columns, SQLAlchemy returns the dict/list directly + # For backward compatibility with Text columns, try to decode if it's a string + if isinstance(self.entity_metadata, str): + return self._decode_value(self.entity_metadata) + return self.entity_metadata + + def get_full_old_state(self): + """Get the decoded full old state""" + return self._decode_value(self.full_old_state) + + def get_full_new_state(self): + """Get the decoded full new state""" + return self._decode_value(self.full_new_state) + @classmethod def get_for_entity(cls, entity_type, entity_id, limit=100): """Get audit logs for a specific entity""" @@ -224,6 +269,10 @@ class AuditLog(db.Model): "old_value": self.get_old_value(), "new_value": self.get_new_value(), "change_description": self.change_description, + "reason": self.reason, + "entity_metadata": self.get_entity_metadata(), + "full_old_state": self.get_full_old_state(), + "full_new_state": self.get_full_new_state(), "ip_address": self.ip_address, "user_agent": self.user_agent, "request_path": self.request_path, diff --git a/app/models/time_entry.py b/app/models/time_entry.py index df633b35..0ec392cd 100644 --- a/app/models/time_entry.py +++ b/app/models/time_entry.py @@ -136,11 +136,15 @@ class TimeEntry(db.Model): @property def duration_formatted(self): """Get duration formatted as HH:MM:SS""" - if not self.duration_seconds: + # For active timers (end_time is None), use current_duration_seconds + if not self.end_time: + total_seconds = self.current_duration_seconds + elif not self.duration_seconds: return "00:00:00" + else: + total_seconds = int(self.duration_seconds) # Convert to int to ensure integer values for formatting - total_seconds = int(self.duration_seconds) hours = total_seconds // 3600 minutes = (total_seconds % 3600) // 60 seconds = total_seconds % 60 diff --git a/app/routes/api.py b/app/routes/api.py index 7f7b4706..f20c36c4 100644 --- a/app/routes/api.py +++ b/app/routes/api.py @@ -1418,15 +1418,7 @@ def update_entry(entry_id): return jsonify({"error": "Access denied"}), 403 data = request.get_json() or {} - - # Optional: project change (admin only) - new_project_id = data.get("project_id") - if new_project_id is not None and current_user.is_admin: - if new_project_id != entry.project_id: - project = Project.query.filter_by(id=new_project_id, status="active").first() - if not project: - return jsonify({"error": "Invalid project"}), 400 - entry.project_id = new_project_id + reason = data.get("reason") # Optional reason for the change # Optional: start/end time updates (admin only for safety) # Accept HTML datetime-local format: YYYY-MM-DDTHH:MM @@ -1445,79 +1437,43 @@ def update_entry(entry_id): except Exception: return None - if current_user.is_admin: - start_time_str = data.get("start_time") - end_time_str = data.get("end_time") - - if start_time_str: - parsed_start = parse_dt_local(start_time_str) - if not parsed_start: - return jsonify({"error": "Invalid start time format"}), 400 - entry.start_time = parsed_start - - if end_time_str is not None: - if end_time_str == "" or end_time_str is False: - entry.end_time = None - entry.duration_seconds = None - else: - parsed_end = parse_dt_local(end_time_str) - if not parsed_end: - return jsonify({"error": "Invalid end time format"}), 400 - if parsed_end <= (entry.start_time or parsed_end): - return jsonify({"error": "End time must be after start time"}), 400 - entry.end_time = parsed_end - # Recalculate duration - entry.calculate_duration() - - # Prevent multiple active timers for the same user when editing - if entry.end_time is None: - conflict = ( - TimeEntry.query.filter(TimeEntry.user_id == entry.user_id) - .filter(TimeEntry.end_time.is_(None)) - .filter(TimeEntry.id != entry.id) - .first() - ) - if conflict: - return jsonify({"error": "User already has an active timer"}), 400 - - # Notes, tags, billable (both admin and owner can change) - if "notes" in data: - entry.notes = data["notes"].strip() if data["notes"] else None - - if "tags" in data: - entry.tags = data["tags"].strip() if data["tags"] else None - - if "billable" in data: - entry.billable = bool(data["billable"]) - - if "paid" in data: - entry.paid = bool(data["paid"]) - # Clear invoice number if marking as unpaid - if not entry.paid: - entry.invoice_number = None - - if "invoice_number" in data: - invoice_number = data["invoice_number"] - entry.invoice_number = invoice_number.strip() if invoice_number else None - - # Prefer local time for updated_at per project preference - entry.updated_at = local_now() - - if not safe_commit("api_update_entry", {"entry_id": entry_id}): - return jsonify({"error": "Database error while updating entry"}), 500 + # Use service layer for update to get enhanced audit logging + from app.services import TimeTrackingService + service = TimeTrackingService() + + # Convert data to service parameters + result = service.update_entry( + entry_id=entry_id, + user_id=current_user.id, + is_admin=current_user.is_admin, + project_id=data.get("project_id") if current_user.is_admin else None, + client_id=data.get("client_id") if current_user.is_admin else None, + task_id=data.get("task_id"), + start_time=parse_dt_local(data.get("start_time")) if current_user.is_admin and data.get("start_time") else None, + end_time=parse_dt_local(data.get("end_time")) if current_user.is_admin and data.get("end_time") else None, + notes=data.get("notes"), + tags=data.get("tags"), + billable=data.get("billable"), + paid=data.get("paid"), + invoice_number=data.get("invoice_number"), + reason=reason, + ) + + if not result.get("success"): + return jsonify({"error": result.get("message", "Could not update entry")}), 400 # Invalidate dashboard cache for the entry owner so changes appear immediately try: from app.utils.cache import get_cache cache = get_cache() - cache_key = f"dashboard:{entry.user_id}" + cache_key = f"dashboard:{result['entry'].user_id}" cache.delete(cache_key) - current_app.logger.debug("Invalidated dashboard cache for user %s after entry update", entry.user_id) + current_app.logger.debug("Invalidated dashboard cache for user %s after entry update", result['entry'].user_id) except Exception as e: current_app.logger.warning("Failed to invalidate dashboard cache: %s", e) - payload = entry.to_dict() - payload["project_name"] = entry.project.name if entry.project else None + payload = result["entry"].to_dict() + payload["project_name"] = result["entry"].project.name if result["entry"].project else None return jsonify({"success": True, "entry": payload}) @@ -1525,50 +1481,33 @@ def update_entry(entry_id): @login_required def delete_entry(entry_id): """Delete a time entry""" - entry = TimeEntry.query.get_or_404(entry_id) - - # Check permissions - if entry.user_id != current_user.id and not current_user.is_admin: - return jsonify({"error": "Access denied"}), 403 - - # Don't allow deletion of active timers - if entry.is_active: - return jsonify({"error": "Cannot delete active timer"}), 400 - - # Capture entry info for logging before deletion - project_name = entry.project.name if entry.project else None - client_name = entry.client.name if entry.client else None - entity_name = project_name or client_name or "Unknown" - duration_formatted = entry.duration_formatted - entry_user_id = entry.user_id # Capture user_id before deletion - - db.session.delete(entry) - db.session.commit() + data = request.get_json() or {} + reason = data.get("reason") # Optional reason for deletion + + # Use service layer for deletion to get enhanced audit logging + from app.services import TimeTrackingService + service = TimeTrackingService() + + result = service.delete_entry( + user_id=current_user.id, + entry_id=entry_id, + is_admin=current_user.is_admin, + reason=reason, + ) + + if not result.get("success"): + return jsonify({"error": result.get("message", "Could not delete entry")}), 400 # Invalidate dashboard cache for the entry owner so changes appear immediately try: from app.utils.cache import get_cache cache = get_cache() - cache_key = f"dashboard:{entry_user_id}" + cache_key = f"dashboard:{current_user.id}" cache.delete(cache_key) - current_app.logger.debug("Invalidated dashboard cache for user %s after entry deletion", entry_user_id) + current_app.logger.debug("Invalidated dashboard cache for user %s after entry deletion", current_user.id) except Exception as e: current_app.logger.warning("Failed to invalidate dashboard cache: %s", e) - # Log activity - from app.models import Activity - Activity.log( - user_id=current_user.id, - action="deleted", - entity_type="time_entry", - entity_id=entry_id, - entity_name=entity_name, - description=f'Deleted time entry for {entity_name} - {duration_formatted}', - extra_data={"project_name": project_name, "client_name": client_name, "duration_formatted": duration_formatted}, - ip_address=request.remote_addr, - user_agent=request.headers.get("User-Agent"), - ) - return jsonify({"success": True}) diff --git a/app/routes/api_v1.py b/app/routes/api_v1.py index 4b1f3d4b..00753dc2 100644 --- a/app/routes/api_v1.py +++ b/app/routes/api_v1.py @@ -716,6 +716,7 @@ def update_time_entry(entry_id): user_id=g.api_user.id, is_admin=g.api_user.is_admin, project_id=data.get("project_id"), + client_id=data.get("client_id"), task_id=data.get("task_id"), start_time=start_time, end_time=end_time, @@ -724,6 +725,7 @@ def update_time_entry(entry_id): billable=data.get("billable"), paid=data.get("paid"), invoice_number=data.get("invoice_number"), + reason=data.get("reason"), ) if not result.get("success"): @@ -754,8 +756,16 @@ def delete_time_entry(entry_id): """ from app.services import TimeTrackingService + data = request.get_json() or {} + reason = data.get("reason") # Optional reason for deletion + time_tracking_service = TimeTrackingService() - result = time_tracking_service.delete_entry(entry_id=entry_id, user_id=g.api_user.id, is_admin=g.api_user.is_admin) + result = time_tracking_service.delete_entry( + entry_id=entry_id, + user_id=g.api_user.id, + is_admin=g.api_user.is_admin, + reason=reason, + ) if not result.get("success"): return jsonify({"error": result.get("message", "Could not delete time entry")}), 400 diff --git a/app/routes/timer.py b/app/routes/timer.py index 28019d39..085b1441 100644 --- a/app/routes/timer.py +++ b/app/routes/timer.py @@ -418,7 +418,7 @@ def stop_timer(): current_app.logger.info("Stopped timer id=%s for user=%s", active_timer.id, current_user.username) # Track timer stopped event - duration_seconds = active_timer.duration if hasattr(active_timer, "duration") else 0 + duration_seconds = active_timer.duration_seconds if active_timer.duration_seconds else 0 log_event( "timer.stopped", user_id=current_user.id, @@ -467,30 +467,37 @@ def stop_timer(): current_user.id, {"source": "timer", "duration_seconds": duration_seconds, "has_task": bool(active_timer.task_id)}, ) + + # Emit WebSocket event for real-time updates + try: + socketio.emit( + "timer_stopped", + {"user_id": current_user.id, "timer_id": active_timer.id, "duration": active_timer.duration_formatted}, + ) + except Exception as e: + current_app.logger.warning("Socket emit failed for timer_stopped: %s", e) + + # Invalidate dashboard cache so timer disappears immediately + try: + from app.utils.cache import get_cache + cache = get_cache() + cache_key = f"dashboard:{current_user.id}" + cache.delete(cache_key) + current_app.logger.debug("Invalidated dashboard cache for user %s", current_user.id) + except Exception as e: + current_app.logger.warning("Failed to invalidate dashboard cache: %s", e) + + flash(f"Timer stopped. Duration: {active_timer.duration_formatted}", "success") + return redirect(url_for("main.dashboard")) + except ValueError as e: + # Timer already stopped or invalid state + current_app.logger.warning("Cannot stop timer: %s", e) + flash(_("Cannot stop timer: %(error)s", error=str(e)), "error") + return redirect(url_for("main.dashboard")) except Exception as e: current_app.logger.exception("Error stopping timer: %s", e) - - # Emit WebSocket event for real-time updates - try: - socketio.emit( - "timer_stopped", - {"user_id": current_user.id, "timer_id": active_timer.id, "duration": active_timer.duration_formatted}, - ) - except Exception as e: - current_app.logger.warning("Socket emit failed for timer_stopped: %s", e) - - # Invalidate dashboard cache so timer disappears immediately - try: - from app.utils.cache import get_cache - cache = get_cache() - cache_key = f"dashboard:{current_user.id}" - cache.delete(cache_key) - current_app.logger.debug("Invalidated dashboard cache for user %s", current_user.id) - except Exception as e: - current_app.logger.warning("Failed to invalidate dashboard cache: %s", e) - - flash(f"Timer stopped. Duration: {active_timer.duration_formatted}", "success") - return redirect(url_for("main.dashboard")) + flash(_("Could not stop timer due to an error. Please try again or contact support if the problem persists."), "error") + return redirect(url_for("main.dashboard")) @timer_bp.route("/timer/status") @@ -529,18 +536,31 @@ def edit_timer(timer_id): return redirect(url_for("main.dashboard")) if request.method == "POST": - # Update timer details - timer.notes = request.form.get("notes", "").strip() - timer.tags = request.form.get("tags", "").strip() - timer.billable = request.form.get("billable") == "on" - timer.paid = request.form.get("paid") == "on" + # Get reason for change + reason = request.form.get("reason", "").strip() or None + + # Use service layer for update to get enhanced audit logging + from app.services import TimeTrackingService + service = TimeTrackingService() + + # Prepare update parameters + update_params = { + "entry_id": timer_id, + "user_id": current_user.id, + "is_admin": current_user.is_admin, + "notes": request.form.get("notes", "").strip() or None, + "tags": request.form.get("tags", "").strip() or None, + "billable": request.form.get("billable") == "on", + "paid": request.form.get("paid") == "on", + "reason": reason, + } # Update invoice number invoice_number = request.form.get("invoice_number", "").strip() - timer.invoice_number = invoice_number if invoice_number else None + update_params["invoice_number"] = invoice_number if invoice_number else None # Clear invoice number if marking as unpaid - if not timer.paid: - timer.invoice_number = None + if update_params["paid"] is False: + update_params["invoice_number"] = None # Admin users can edit additional fields if current_user.is_admin: @@ -549,7 +569,7 @@ def edit_timer(timer_id): if new_project_id and new_project_id != timer.project_id: new_project = Project.query.filter_by(id=new_project_id, status="active").first() if new_project: - timer.project_id = new_project_id + update_params["project_id"] = new_project_id else: flash(_("Invalid project selected"), "error") return render_template( @@ -562,14 +582,16 @@ def edit_timer(timer_id): else Task.query.filter_by(project_id=new_project_id).order_by(Task.name).all() ), ) + else: + update_params["project_id"] = None # Don't change if not provided # Update task if changed new_task_id = request.form.get("task_id", type=int) if new_task_id != timer.task_id: if new_task_id: - new_task = Task.query.filter_by(id=new_task_id, project_id=timer.project_id).first() + new_task = Task.query.filter_by(id=new_task_id, project_id=update_params.get("project_id") or timer.project_id).first() if new_task: - timer.task_id = new_task_id + update_params["task_id"] = new_task_id else: flash(_("Invalid task selected for the chosen project"), "error") return render_template( @@ -579,7 +601,9 @@ def edit_timer(timer_id): tasks=Task.query.filter_by(project_id=timer.project_id).order_by(Task.name).all(), ) else: - timer.task_id = None + update_params["task_id"] = None + else: + update_params["task_id"] = None # Don't change if not provided # Update start and end times if provided start_date = request.form.get("start_date") @@ -606,7 +630,7 @@ def edit_timer(timer_id): tasks=Task.query.filter_by(project_id=timer.project_id).order_by(Task.name).all(), ) - timer.start_time = new_start_time + update_params["start_time"] = new_start_time except ValueError: flash(_("Invalid start date/time format"), "error") return render_template( @@ -615,6 +639,8 @@ def edit_timer(timer_id): projects=Project.query.filter_by(status="active").order_by(Project.name).all(), tasks=Task.query.filter_by(project_id=timer.project_id).order_by(Task.name).all(), ) + else: + update_params["start_time"] = None if end_date and end_time: try: @@ -623,7 +649,8 @@ def edit_timer(timer_id): new_end_time = utc_to_local(parsed_end_utc).replace(tzinfo=None) # Validate that end time is after start time - if new_end_time <= timer.start_time: + start_time_for_validation = update_params.get("start_time") or timer.start_time + if new_end_time <= start_time_for_validation: flash(_("End time must be after start time"), "error") return render_template( "timer/edit_timer.html", @@ -632,9 +659,7 @@ def edit_timer(timer_id): tasks=Task.query.filter_by(project_id=timer.project_id).order_by(Task.name).all(), ) - timer.end_time = new_end_time - # Recalculate duration - timer.calculate_duration() + update_params["end_time"] = new_end_time except ValueError: flash(_("Invalid end date/time format"), "error") return render_template( @@ -643,15 +668,20 @@ def edit_timer(timer_id): projects=Project.query.filter_by(status="active").order_by(Project.name).all(), tasks=Task.query.filter_by(project_id=timer.project_id).order_by(Task.name).all(), ) + else: + update_params["end_time"] = None - # Update source if provided - new_source = request.form.get("source") - if new_source in ["manual", "auto"]: - timer.source = new_source - - if not safe_commit("edit_timer", {"timer_id": timer.id}): - flash(_("Could not update timer due to a database error. Please check server logs."), "error") - return redirect(url_for("main.dashboard")) + # Call service layer to update + result = service.update_entry(**update_params) + + if not result.get("success"): + flash(_(result.get("message", "Could not update timer")), "error") + return render_template( + "timer/edit_timer.html", + timer=timer, + projects=Project.query.filter_by(status="active").order_by(Project.name).all() if current_user.is_admin else [], + tasks=Task.query.filter_by(project_id=timer.project_id).order_by(Task.name).all() if current_user.is_admin and timer.project_id else [], + ) # Invalidate dashboard cache for the timer owner so changes appear immediately try: @@ -830,9 +860,10 @@ def delete_timer(timer_id): @login_required def bulk_delete_time_entries(): """Bulk delete time entries""" - from app.utils.db import safe_commit + from app.services import TimeTrackingService entry_ids = request.form.getlist("entry_ids[]") + reason = request.form.get("reason", "").strip() or None # Optional reason for bulk deletion if not entry_ids: flash(_("No time entries selected"), "warning") @@ -855,6 +886,9 @@ def bulk_delete_time_entries(): deleted_count = 0 skipped_count = 0 + # Use service layer for proper audit logging + service = TimeTrackingService() + for entry in entries: # Check permissions if not can_view_all and entry.user_id != current_user.id: @@ -866,28 +900,20 @@ def bulk_delete_time_entries(): skipped_count += 1 continue - # Delete the entry - db.session.delete(entry) - deleted_count += 1 - - # Log activity - Activity.log( + # Delete using service layer to get enhanced audit logging + result = service.delete_entry( user_id=current_user.id, - action="deleted", - entity_type="time_entry", - entity_id=entry.id, - entity_name=f"Time entry #{entry.id}", - description=f"Deleted time entry", - extra_data={"project_id": entry.project_id, "client_id": entry.client_id, "duration": entry.duration_formatted}, - ip_address=request.remote_addr, - user_agent=request.headers.get("User-Agent"), + entry_id=entry.id, + is_admin=current_user.is_admin, + reason=reason, # Use same reason for all entries in bulk delete ) + + if result.get("success"): + deleted_count += 1 + else: + skipped_count += 1 if deleted_count > 0: - if not safe_commit("bulk_delete_time_entries", {"count": deleted_count}): - flash(_("Could not delete time entries due to a database error. Please check server logs."), "error") - return redirect(url_for("timer.time_entries_overview")) - flash( _("Successfully deleted %(count)d time entry/entries", count=deleted_count), "success" @@ -1105,6 +1131,16 @@ def manual_entry(): else: flash(_("Manual entry created for %(target)s", target=target_name), "success") + # Invalidate dashboard cache so new entry appears immediately + try: + from app.utils.cache import get_cache + cache = get_cache() + cache_key = f"dashboard:{current_user.id}" + cache.delete(cache_key) + current_app.logger.debug("Invalidated dashboard cache for user %s after manual entry creation", current_user.id) + except Exception as e: + current_app.logger.warning("Failed to invalidate dashboard cache: %s", e) + return redirect(url_for("main.dashboard")) return render_template( diff --git a/app/services/time_tracking_service.py b/app/services/time_tracking_service.py index 1f5d47f0..c7f3238a 100644 --- a/app/services/time_tracking_service.py +++ b/app/services/time_tracking_service.py @@ -301,10 +301,27 @@ class TimeTrackingService: billable: Optional[bool] = None, paid: Optional[bool] = None, invoice_number: Optional[str] = None, + reason: Optional[str] = None, ) -> Dict[str, Any]: """ Update a time entry. + Args: + entry_id: ID of the time entry to update + user_id: ID of the user performing the update + is_admin: Whether the user is an admin + project_id: Optional new project ID + client_id: Optional new client ID + task_id: Optional new task ID + start_time: Optional new start time + end_time: Optional new end time + notes: Optional new notes + tags: Optional new tags + billable: Optional new billable status + paid: Optional new paid status + invoice_number: Optional new invoice number + reason: Optional reason for the change + Returns: dict with 'success', 'message', and 'entry' keys """ @@ -325,6 +342,11 @@ class TimeTrackingService: "error": "timer_active", } + # Capture old state before changes + from app.utils.audit import capture_timeentry_state, capture_timeentry_metadata + full_old_state = capture_timeentry_state(entry) + entity_metadata = capture_timeentry_metadata(entry) + # Update fields if project_id is not None: # Validate project @@ -384,12 +406,52 @@ class TimeTrackingService: "error": "database_error", } + # Capture new state after changes and create comprehensive audit log + try: + # Refresh entry to get updated values + db.session.refresh(entry) + full_new_state = capture_timeentry_state(entry) + updated_metadata = capture_timeentry_metadata(entry) + + from app.models.audit_log import AuditLog + from app.utils.audit import get_request_info + ip_address, user_agent, request_path = get_request_info() + + entity_name = entry.project.name if entry.project else (entry.client.name if entry.client else "Unknown") + + AuditLog.log_change( + user_id=user_id, + action="updated", + entity_type="TimeEntry", + entity_id=entry_id, + entity_name=entity_name, + change_description=f"Updated time entry for {entity_name}", + reason=reason, + entity_metadata=updated_metadata, + full_old_state=full_old_state, + full_new_state=full_new_state, + ip_address=ip_address, + user_agent=user_agent, + request_path=request_path, + ) + db.session.commit() + except Exception as e: + # Don't fail update if audit logging fails + import logging + logging.getLogger(__name__).warning(f"Failed to create audit log for TimeEntry update: {e}") + return {"success": True, "message": "Time entry updated successfully", "entry": entry} - def delete_entry(self, user_id: int, entry_id: int, is_admin: bool = False) -> Dict[str, Any]: + def delete_entry(self, user_id: int, entry_id: int, is_admin: bool = False, reason: Optional[str] = None) -> Dict[str, Any]: """ Delete a time entry. + Args: + user_id: ID of the user performing the deletion + entry_id: ID of the time entry to delete + is_admin: Whether the user is an admin + reason: Optional reason for deletion + Returns: dict with 'success' and 'message' keys """ @@ -416,11 +478,41 @@ class TimeTrackingService: entity_name = project_name or client_name or "Unknown" duration_formatted = entry.duration_formatted + # Capture full state and metadata for audit logging + from app.utils.audit import capture_timeentry_state, capture_timeentry_metadata, get_request_info + from app.models.audit_log import AuditLog + + full_old_state = capture_timeentry_state(entry) + entity_metadata = capture_timeentry_metadata(entry) + ip_address, user_agent, request_path = get_request_info() + if self.time_entry_repo.delete(entry): if safe_commit("delete_entry", {"user_id": user_id, "entry_id": entry_id}): + # Create comprehensive audit log entry + try: + AuditLog.log_change( + user_id=user_id, + action="deleted", + entity_type="TimeEntry", + entity_id=entry_id, + entity_name=entity_name, + change_description=f"Deleted time entry for {entity_name} - {duration_formatted}", + reason=reason, + entity_metadata=entity_metadata, + full_old_state=full_old_state, + ip_address=ip_address, + user_agent=user_agent, + request_path=request_path, + ) + db.session.commit() + except Exception as e: + # Don't fail deletion if audit logging fails + import logging + logging.getLogger(__name__).warning(f"Failed to create audit log for TimeEntry deletion: {e}") + # Log activity from app.models import Activity - from flask import request, has_request_context + from flask import request Activity.log( user_id=user_id, action="deleted", @@ -428,9 +520,9 @@ class TimeTrackingService: entity_id=entry_id, entity_name=entity_name, description=f'Deleted time entry for {entity_name} - {duration_formatted}', - extra_data={"project_name": project_name, "client_name": client_name, "duration_formatted": duration_formatted}, - ip_address=request.remote_addr if has_request_context() else None, - user_agent=request.headers.get("User-Agent") if has_request_context() else None, + extra_data={"project_name": project_name, "client_name": client_name, "duration_formatted": duration_formatted, "reason": reason}, + ip_address=ip_address, + user_agent=user_agent, ) return {"success": True, "message": "Time entry deleted successfully"} diff --git a/app/templates/audit_logs/list.html b/app/templates/audit_logs/list.html index 138cd834..dd1c90e1 100644 --- a/app/templates/audit_logs/list.html +++ b/app/templates/audit_logs/list.html @@ -97,7 +97,7 @@ {{ badge(log.action, log.get_color()) }} - + {{ log.entity_type }}#{{ log.entity_id }} @@ -105,6 +105,30 @@ {% if log.entity_name %}
{{ log.entity_name }} {% endif %} + {% if log.entity_type == 'TimeEntry' and log.get_entity_metadata() %} + {% set metadata = log.get_entity_metadata() %} +
+ {% if metadata.project_name %} +
{{ _('Project') }}: {{ metadata.project_name }}{% if metadata.project_id %} (ID: {{ metadata.project_id }}){% endif %}
+ {% elif metadata.client_name %} +
{{ _('Client') }}: {{ metadata.client_name }}{% if metadata.client_id %} (ID: {{ metadata.client_id }}){% endif %}
+ {% endif %} + {% if metadata.created_at %} +
{{ _('Created') }}: + {% if metadata.created_at is string %} + {{ metadata.created_at[:16]|replace('T', ' ') }} + {% else %} + {{ metadata.created_at|user_datetime('%Y-%m-%d %H:%M') }} + {% endif %} +
+ {% endif %} +
+ {% endif %} + {% if log.reason %} +
+ {{ _('Reason') }}: {{ log.reason }} +
+ {% endif %} {% if log.field_name %} diff --git a/app/templates/audit_logs/view.html b/app/templates/audit_logs/view.html index 40b16cc0..21f0b410 100644 --- a/app/templates/audit_logs/view.html +++ b/app/templates/audit_logs/view.html @@ -71,13 +71,52 @@ {% endif %} + {% if audit_log.reason %} +
+
{{ _('Reason') }}
+
+ {{ audit_log.reason }} +
+
+ {% endif %} + {% if audit_log.entity_type == 'TimeEntry' and audit_log.get_entity_metadata() %} + {% set metadata = audit_log.get_entity_metadata() %} +
+
{{ _('Entity Context') }}
+
+
+ {% if metadata.project_name %} +
{{ _('Project') }}: {{ metadata.project_name }}{% if metadata.project_id %} (ID: {{ metadata.project_id }}){% endif %}
+ {% endif %} + {% if metadata.client_name %} +
{{ _('Client') }}: {{ metadata.client_name }}{% if metadata.client_id %} (ID: {{ metadata.client_id }}){% endif %}
+ {% endif %} + {% if metadata.task_name %} +
{{ _('Task') }}: {{ metadata.task_name }}{% if metadata.task_id %} (ID: {{ metadata.task_id }}){% endif %}
+ {% endif %} + {% if metadata.created_at %} +
{{ _('TimeEntry Created') }}: + {% if metadata.created_at is string %} + {{ metadata.created_at[:19]|replace('T', ' ') }} + {% else %} + {{ metadata.created_at|user_datetime('%Y-%m-%d %H:%M:%S') }} + {% endif %} +
+ {% endif %} + {% if metadata.user_name %} +
{{ _('Entry Owner') }}: {{ metadata.user_name }}{% if metadata.user_id %} (ID: {{ metadata.user_id }}){% endif %}
+ {% endif %} +
+
+
+ {% endif %} {% if audit_log.field_name %}
-

{{ _('Change Details') }}

+

{{ _('Field Change Details') }}

{% if audit_log.old_value %}
@@ -99,6 +138,41 @@
{% endif %} + + {% if audit_log.get_full_old_state() or audit_log.get_full_new_state() %} +
+

{{ _('Full Entity State') }}

+
+ {% if audit_log.get_full_old_state() %} +
+

{{ _('Before Change') }}

+
+ {% set old_state = audit_log.get_full_old_state() %} + {% if old_state is mapping or old_state is iterable and old_state is not string %} + 
{{ old_state|tojson(indent=2) }}
+ {% else %} + 
{{ old_state }}
+ {% endif %} +
+
+ {% endif %} + {% if audit_log.get_full_new_state() %} +
+

{{ _('After Change') }}

+
+ {% set new_state = audit_log.get_full_new_state() %} + {% if new_state is mapping or new_state is iterable and new_state is not string %} + 
{{ new_state|tojson(indent=2) }}
+ {% else %} + 
{{ new_state }}
+ {% endif %} +
+
+ {% endif %} +
+
+ {% endif %} +

{{ _('Request Information') }}

diff --git a/app/templates/timer/time_entries_overview.html b/app/templates/timer/time_entries_overview.html index 663e40bc..2c435d40 100644 --- a/app/templates/timer/time_entries_overview.html +++ b/app/templates/timer/time_entries_overview.html @@ -119,6 +119,7 @@
+ + + + +
+ + +

{{ _('Explain why you are making these changes') }}

+
+
@@ -441,6 +450,15 @@ document.addEventListener('DOMContentLoaded', function() {
+ +
+ + +

{{ _('Explain why you are making these changes') }}

+
+