feat(time-approvals): complete time entry approval workflow and fix bugs

- Fix approve crash: make _mark_entry_approved a no-op (approval state from
  TimeEntryApproval only; TimeEntry has no metadata column).
- Fix PostgreSQL enum: use values_callable on ApprovalStatus so DB receives
  'pending' not 'PENDING' (matches approvalstatus enum).
- Fix approval templates: use requester/approver, request_comment,
  time_entry.notes; reject form field name 'reason'; add can_approve to
  view and pass from route.
- Filter get_pending_approvals by current approver (policy/project fallback).
- Add Time Approvals nav link under Work in base.html (module-enabled).
- Add Request approval in time entries list (icon) and view_timer (sidebar);
  redirect to list_approvals after request.
- Fix empty_state calls in approvals/list.html (use icon_class positional
  args to match components/ui.html macro).
This commit is contained in:
Dries Peeters
2026-02-13 21:06:18 +01:00
parent ab14e4352a
commit 115db52b2b
8 changed files with 97 additions and 46 deletions
+7 -2
View File
@@ -25,8 +25,13 @@ class TimeEntryApproval(db.Model):
id = db.Column(db.Integer, primary_key=True)
time_entry_id = db.Column(db.Integer, db.ForeignKey("time_entries.id"), nullable=False, index=True)
# Approval workflow
status = db.Column(SQLEnum(ApprovalStatus), default=ApprovalStatus.PENDING, nullable=False, index=True)
# Approval workflow (use enum value for PostgreSQL: 'pending' not 'PENDING')
status = db.Column(
SQLEnum(ApprovalStatus, values_callable=lambda x: [e.value for e in x]),
default=ApprovalStatus.PENDING,
nullable=False,
index=True,
)
requested_by = db.Column(db.Integer, db.ForeignKey("users.id"), nullable=False, index=True)
approved_by = db.Column(db.Integer, db.ForeignKey("users.id"), nullable=True, index=True)
+10 -4
View File
@@ -40,14 +40,20 @@ def view_approval(approval_id):
approval = TimeEntryApproval.query.get_or_404(approval_id)
# Check permissions
service = TimeApprovalService()
approver_ids = service._get_approvers_for_entry(approval.time_entry)
if approval.requested_by != current_user.id and approval.approved_by != current_user.id:
service = TimeApprovalService()
approver_ids = service._get_approvers_for_entry(approval.time_entry)
if current_user.id not in approver_ids and not current_user.is_admin:
flash(_("Access denied"), "error")
return redirect(url_for("time_approvals.list_approvals"))
return render_template("approvals/view.html", approval=approval)
# Can current user approve this (pending) request?
can_approve = (
approval.status == ApprovalStatus.PENDING
and (current_user.id in approver_ids or current_user.is_admin)
)
return render_template("approvals/view.html", approval=approval, can_approve=can_approve)
@time_approvals_bp.route("/approvals/<int:approval_id>/approve", methods=["POST"])
@@ -122,7 +128,7 @@ def request_approval(entry_id):
else:
flash(_(result.get("message", "Request failed")), "error")
return redirect(url_for("main.dashboard"))
return redirect(url_for("time_approvals.list_approvals"))
@time_approvals_bp.route("/approvals/<int:approval_id>/cancel", methods=["POST"])
+18 -18
View File
@@ -146,17 +146,21 @@ class TimeApprovalService:
return {"success": True, "message": "Approval cancelled", "approval": approval.to_dict()}
def get_pending_approvals(self, approver_id: int = None) -> List[TimeEntryApproval]:
"""Get pending approvals for an approver"""
query = TimeEntryApproval.query.filter_by(status=ApprovalStatus.PENDING)
if approver_id:
# Get approvals where user is an approver
approver_ids = self._get_all_approver_ids(approver_id)
# This would need a more sophisticated query in production
# For now, return all pending approvals
pass
return query.order_by(TimeEntryApproval.requested_at.desc()).all()
"""Get pending approvals for an approver. When approver_id is set, only return approvals this user may approve."""
all_pending = (
TimeEntryApproval.query.filter_by(status=ApprovalStatus.PENDING)
.order_by(TimeEntryApproval.requested_at.desc())
.all()
)
if not approver_id:
return all_pending
# Filter to approvals where this user is a valid approver for the time entry
result = []
for approval in all_pending:
entry_approvers = self._get_approvers_for_entry(approval.time_entry)
if approver_id in entry_approvers:
result.append(approval)
return result
def bulk_approve(self, approval_ids: List[int], approver_id: int, comment: str = None) -> Dict[str, Any]:
"""Bulk approve multiple time entries"""
@@ -213,13 +217,9 @@ class TimeApprovalService:
return approver_ids
def _mark_entry_approved(self, time_entry: TimeEntry):
"""Mark time entry as approved"""
# Add metadata to indicate approval
if not hasattr(time_entry, "metadata") or not time_entry.metadata:
time_entry.metadata = {}
time_entry.metadata["approved"] = True
time_entry.metadata["approved_at"] = datetime.utcnow().isoformat()
db.session.commit()
"""Mark time entry as approved. Approval state is derived from TimeEntryApproval records; no column on TimeEntry."""
# No-op: approved status is determined by existence of an approved TimeEntryApproval for this entry.
pass
def _notify_approvers(self, approval: TimeEntryApproval, approver_ids: List[int]):
"""Send notifications to approvers"""
+10 -10
View File
@@ -34,7 +34,7 @@
</a>
</h3>
<p class="text-sm text-text-muted-light dark:text-text-muted-dark">
{{ _('Requested by') }}: {{ approval.requested_by_user.username if approval.requested_by_user else 'N/A' }} •
{{ _('Requested by') }}: {{ approval.requester.username if approval.requester else 'N/A' }} •
{{ approval.time_entry.duration_hours|round(2) }} {{ _('hours') }}
</p>
{% if approval.time_entry.project %}
@@ -59,10 +59,10 @@
</button>
</div>
</div>
{% if approval.comments %}
{% if approval.request_comment %}
<div class="mt-3 pt-3 border-t border-border-light dark:border-border-dark">
<p class="text-sm text-text-muted-light dark:text-text-muted-dark">
<i class="fas fa-comment mr-1"></i>{{ approval.comments }}
<i class="fas fa-comment mr-1"></i>{{ approval.request_comment }}
</p>
</div>
{% endif %}
@@ -71,9 +71,9 @@
</div>
{% else %}
{{ empty_state(
icon='fas fa-check-circle',
title=_('No pending approvals'),
message=_('All time entries have been reviewed.')
'fas fa-check-circle',
_('No pending approvals'),
_('All time entries have been reviewed.')
) }}
{% endif %}
</div>
@@ -112,9 +112,9 @@
</div>
{% else %}
{{ empty_state(
icon='fas fa-inbox',
title=_('No pending requests'),
message=_('You have no time entry approval requests.')
'fas fa-inbox',
_('No pending requests'),
_('You have no time entry approval requests.')
) }}
{% endif %}
</div>
@@ -132,7 +132,7 @@
<input type="hidden" name="csrf_token" value="{{ csrf_token() }}">
<div class="mb-4">
<label for="rejectReason" class="block text-sm font-medium mb-2">{{ _('Reason for rejection') }}</label>
<textarea id="rejectReason" name="comments" rows="4" class="form-input w-full" required></textarea>
<textarea id="rejectReason" name="reason" rows="4" class="form-input w-full" required></textarea>
</div>
<div class="flex justify-end gap-2">
<button type="button" onclick="closeRejectModal()" class="px-4 py-2 rounded-lg border border-border-light dark:border-border-dark hover:bg-background-light dark:hover:bg-background-dark">{{ _('Cancel') }}</button>
+23 -11
View File
@@ -57,10 +57,10 @@
<dt class="text-sm font-medium text-text-muted-light dark:text-text-muted-dark">{{ _('Date') }}</dt>
<dd class="text-text-light dark:text-text-dark">{{ approval.time_entry.start_time|local_date if approval.time_entry.start_time else 'N/A' }}</dd>
</div>
{% if approval.time_entry.description %}
{% if approval.time_entry.notes %}
<div>
<dt class="text-sm font-medium text-text-muted-light dark:text-text-muted-dark">{{ _('Description') }}</dt>
<dd class="text-text-light dark:text-text-dark">{{ approval.time_entry.description }}</dd>
<dt class="text-sm font-medium text-text-muted-light dark:text-text-muted-dark">{{ _('Notes') }}</dt>
<dd class="text-text-light dark:text-text-dark">{{ approval.time_entry.notes }}</dd>
</div>
{% endif %}
</dl>
@@ -84,16 +84,16 @@
</div>
<div>
<dt class="text-sm font-medium text-text-muted-light dark:text-text-muted-dark">{{ _('Requested by') }}</dt>
<dd class="text-text-light dark:text-text-dark">{{ approval.requested_by_user.username if approval.requested_by_user else 'N/A' }}</dd>
<dd class="text-text-light dark:text-text-dark">{{ approval.requester.username if approval.requester else 'N/A' }}</dd>
</div>
<div>
<dt class="text-sm font-medium text-text-muted-light dark:text-text-muted-dark">{{ _('Requested at') }}</dt>
<dd class="text-text-light dark:text-text-dark">{{ approval.requested_at|local_datetime if approval.requested_at else 'N/A' }}</dd>
</div>
{% if approval.approved_by_user %}
{% if approval.approver %}
<div>
<dt class="text-sm font-medium text-text-muted-light dark:text-text-muted-dark">{{ _('Approved by') }}</dt>
<dd class="text-text-light dark:text-text-dark">{{ approval.approved_by_user.username }}</dd>
<dd class="text-text-light dark:text-text-dark">{{ approval.approver.username }}</dd>
</div>
{% endif %}
{% if approval.approved_at %}
@@ -102,17 +102,29 @@
<dd class="text-text-light dark:text-text-dark">{{ approval.approved_at|local_datetime }}</dd>
</div>
{% endif %}
{% if approval.comments %}
{% if approval.request_comment %}
<div>
<dt class="text-sm font-medium text-text-muted-light dark:text-text-muted-dark">{{ _('Comments') }}</dt>
<dd class="text-text-light dark:text-text-dark">{{ approval.comments }}</dd>
<dt class="text-sm font-medium text-text-muted-light dark:text-text-muted-dark">{{ _('Request comment') }}</dt>
<dd class="text-text-light dark:text-text-dark">{{ approval.request_comment }}</dd>
</div>
{% endif %}
{% if approval.approval_comment %}
<div>
<dt class="text-sm font-medium text-text-muted-light dark:text-text-muted-dark">{{ _('Approval comment') }}</dt>
<dd class="text-text-light dark:text-text-dark">{{ approval.approval_comment }}</dd>
</div>
{% endif %}
{% if approval.rejection_reason %}
<div>
<dt class="text-sm font-medium text-text-muted-light dark:text-text-muted-dark">{{ _('Rejection reason') }}</dt>
<dd class="text-text-light dark:text-text-dark">{{ approval.rejection_reason }}</dd>
</div>
{% endif %}
</dl>
</div>
</div>
{% if approval.status.value == 'pending' and (approval.approved_by == current_user.id or current_user.is_admin) %}
{% if approval.status.value == 'pending' and can_approve %}
<div class="mt-6 pt-6 border-t border-border-light dark:border-border-dark flex gap-3">
<form method="POST" action="{{ url_for('time_approvals.approve_entry', approval_id=approval.id) }}" class="inline">
<input type="hidden" name="csrf_token" value="{{ csrf_token() }}">
@@ -139,7 +151,7 @@
<input type="hidden" name="csrf_token" value="{{ csrf_token() }}">
<div class="mb-4">
<label for="rejectReason" class="block text-sm font-medium mb-2">{{ _('Reason for rejection') }}</label>
<textarea id="rejectReason" name="comments" rows="4" class="form-input w-full" required></textarea>
<textarea id="rejectReason" name="reason" rows="4" class="form-input w-full" required></textarea>
</div>
<div class="flex justify-end gap-2">
<button type="button" onclick="closeRejectModal()" class="px-4 py-2 rounded-lg border border-border-light dark:border-border-dark hover:bg-background-light dark:hover:bg-background-dark">{{ _('Cancel') }}</button>
+9 -1
View File
@@ -292,7 +292,7 @@
</div>
<nav class="flex-1">
{% set ep = request.endpoint or '' %}
{% set work_open = ep.startswith('projects.') or ep.startswith('tasks.') or ep.startswith('issues.') or ep.startswith('timer.') or ep.startswith('kanban.') or ep.startswith('weekly_goals.') or ep.startswith('project_templates.') or ep.startswith('gantt.') %}
{% set work_open = ep.startswith('projects.') or ep.startswith('tasks.') or ep.startswith('issues.') or ep.startswith('timer.') or ep.startswith('time_approvals.') or ep.startswith('kanban.') or ep.startswith('weekly_goals.') or ep.startswith('project_templates.') or ep.startswith('gantt.') %}
{% set calendar_open = ep.startswith('calendar.') %}
{% set finance_open = ep.startswith('reports.') or ep.startswith('invoices.') or ep.startswith('recurring_invoices.') or ep.startswith('payments.') or ep.startswith('expenses.') or ep.startswith('mileage.') or ep.startswith('per_diem.') or ep.startswith('budget_alerts.') or ep.startswith('invoice_approvals.') or ep.startswith('payment_gateways.') or ep.startswith('scheduled_reports.') or ep.startswith('custom_reports.') %}
{% set crm_open = ep.startswith('clients.') or ep.startswith('quotes.') or ep.startswith('contacts.') or ep.startswith('deals.') or ep.startswith('leads.') %}
@@ -351,6 +351,7 @@
<ul id="workDropdown" class="{% if not work_open %}hidden {% endif %}mt-2 space-y-2 ml-6">
{% set nav_active_timer = ep.startswith('timer.manual') %}
{% set nav_active_time_entries = ep == 'timer.time_entries_overview' %}
{% set nav_active_time_approvals = ep.startswith('time_approvals.') %}
{% set nav_active_projects = ep.startswith('projects.') %}
{% set nav_active_clients = ep.startswith('clients.') %}
{% set nav_active_quotes = ep.startswith('quotes.') %}
@@ -370,6 +371,13 @@
<i class="fas fa-list-alt w-4 mr-2"></i>{{ _('Time Entries') }}
</a>
</li>
{% if is_module_enabled('time_approvals') %}
<li>
<a class="block px-2 py-1 rounded {% if nav_active_time_approvals %}text-primary font-semibold bg-background-light dark:bg-background-dark{% else %}text-text-light dark:text-text-dark hover:bg-background-light dark:hover:bg-background-dark{% endif %}" href="{{ url_for('time_approvals.list_approvals') }}">
<i class="fas fa-check-double w-4 mr-2"></i>{{ _('Time Approvals') }}
</a>
</li>
{% endif %}
<li>
<a class="block px-2 py-1 rounded {% if nav_active_projects %}text-primary font-semibold bg-background-light dark:bg-background-dark{% else %}text-text-light dark:text-text-dark hover:bg-background-light dark:hover:bg-background-dark{% endif %}" href="{{ url_for('projects.list_projects') }}">
<i class="fas fa-folder w-4 mr-2"></i>{{ _('Projects') }}
@@ -158,6 +158,14 @@
<a href="{{ url_for('timer.edit_timer', timer_id=entry.id) }}" class="text-primary hover:text-primary/80" title="{{ _('Edit') }}">
<i class="fas fa-edit"></i>
</a>
{% if (time_approvals_enabled|default(false)) and entry.end_time and entry.user_id == current_user.id and entry.id not in (entry_ids_with_pending_approval|default([])) %}
<form method="POST" action="{{ url_for('time_approvals.request_approval', entry_id=entry.id) }}" class="inline" title="{{ _('Request approval') }}">
<input type="hidden" name="csrf_token" value="{{ csrf_token() }}">
<button type="submit" class="text-primary hover:text-primary/80 bg-transparent border-none cursor-pointer p-0" title="{{ _('Request approval') }}">
<i class="fas fa-check-double"></i>
</button>
</form>
{% endif %}
</div>
</td>
</tr>
+12
View File
@@ -99,6 +99,18 @@
</div>
</div>
{% if time_approvals_enabled|default(false) and can_request_approval|default(false) %}
<div class="bg-card-light dark:bg-card-dark p-6 rounded-lg shadow">
<h2 class="text-lg font-semibold mb-4">{{ _('Approval') }}</h2>
<form method="POST" action="{{ url_for('time_approvals.request_approval', entry_id=timer.id) }}">
<input type="hidden" name="csrf_token" value="{{ csrf_token() }}">
<button type="submit" class="w-full px-4 py-2 bg-primary text-white rounded-lg hover:bg-primary/90 transition-colors">
<i class="fas fa-check-double mr-2"></i>{{ _('Request approval') }}
</button>
</form>
</div>
{% endif %}
<div class="bg-card-light dark:bg-card-dark p-6 rounded-lg shadow">
<h2 class="text-lg font-semibold mb-4">{{ _('Status & Billing') }}</h2>
<div class="space-y-4">