From 9547937be2ad87c53527ee79ff924380349626a2 Mon Sep 17 00:00:00 2001 From: Dries Peeters Date: Sun, 15 Mar 2026 09:36:37 +0100 Subject: [PATCH 1/4] docs: update README and guides, add audit and strategy docs - Simplify README version section and point to CHANGELOG - Update UI overview with Reports and installation reference - Refresh CONTRIBUTING, DEVELOPMENT, API.md links/consistency - Add ARCHITECTURE_AUDIT, DOCS_AUDIT, PRODUCT_UX_AUDIT, FRONTEND, PERFORMANCE - Add API_CONSISTENCY_AUDIT, RESPONSE_FORMAT, CONTRIBUTOR_GUIDE, TESTING_STRATEGY - Update GETTING_STARTED, REST_API, PROJECT_STRUCTURE, DEPLOYMENT_GUIDE --- API.md | 1 + CONTRIBUTING.md | 1 + DEVELOPMENT.md | 2 +- README.md | 16 +-- docs/ARCHITECTURE_AUDIT.md | 64 ++++++++++ docs/DOCS_AUDIT.md | 73 +++++++++++ docs/FEATURES_COMPLETE.md | 4 +- docs/FRONTEND.md | 70 +++++++++++ docs/GETTING_STARTED.md | 4 +- docs/PERFORMANCE.md | 72 +++++++++++ docs/PRODUCT_UX_AUDIT.md | 129 ++++++++++++++++++++ docs/README.md | 14 ++- docs/admin/deployment/VERSION_MANAGEMENT.md | 2 + docs/api/API_CONSISTENCY_AUDIT.md | 45 +++++++ docs/api/README.md | 3 +- docs/api/RESPONSE_FORMAT.md | 21 ++++ docs/api/REST_API.md | 25 +++- docs/development/CONTRIBUTING.md | 9 +- docs/development/CONTRIBUTOR_GUIDE.md | 93 ++++++++++++++ docs/development/PROJECT_STRUCTURE.md | 21 ++-- docs/guides/DEPLOYMENT_GUIDE.md | 2 + docs/testing/TESTING_STRATEGY.md | 65 ++++++++++ 22 files changed, 706 insertions(+), 30 deletions(-) create mode 100644 docs/ARCHITECTURE_AUDIT.md create mode 100644 docs/DOCS_AUDIT.md create mode 100644 docs/FRONTEND.md create mode 100644 docs/PERFORMANCE.md create mode 100644 docs/PRODUCT_UX_AUDIT.md create mode 100644 docs/api/API_CONSISTENCY_AUDIT.md create mode 100644 docs/api/RESPONSE_FORMAT.md create mode 100644 docs/development/CONTRIBUTOR_GUIDE.md create mode 100644 docs/testing/TESTING_STRATEGY.md diff --git a/API.md b/API.md index abfdeebc..b44003b3 100644 --- a/API.md +++ b/API.md @@ -72,5 +72,6 @@ Replace `your-domain.com` with your TimeTracker host and `YOUR_API_TOKEN` with y ## Full Documentation - **[REST API reference](docs/api/REST_API.md)** — All endpoints, request/response formats, pagination, errors +- **[API Consistency Audit](docs/api/API_CONSISTENCY_AUDIT.md)** — Response contracts, error format, pagination - **[API Token Scopes](docs/api/API_TOKEN_SCOPES.md)** — Scopes and permissions - **[API Versioning](docs/api/API_VERSIONING.md)** — Versioning policy and usage diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index df24fffe..2fe68d3d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -12,6 +12,7 @@ Thank you for your interest in contributing to TimeTracker. This page gives you For development setup, coding standards, testing, pull request process, and commit conventions, see: +- **[Contributor Guide](docs/development/CONTRIBUTOR_GUIDE.md)** — Architecture, local dev, testing, how to add routes/services/templates, versioning - **[Contributing guidelines (full)](docs/development/CONTRIBUTING.md)** — Development setup, coding standards, testing, PR process - **[Code of Conduct](docs/development/CODE_OF_CONDUCT.md)** — Community standards and expected behavior - **[CHANGELOG.md](CHANGELOG.md)** — How we track changes; update the *Unreleased* section for user-facing changes diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 4f968b06..ac35c68b 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -1,6 +1,6 @@ # TimeTracker Development Guide -Quick reference for running the project locally, running tests, and contributing. For full guidelines, see [Contributing](CONTRIBUTING.md) and the [developer documentation](docs/development/CONTRIBUTING.md). +Quick reference for running the project locally, running tests, and contributing. For a single-page contributor overview (workflows, adding routes/services/templates), see [Contributor Guide](docs/development/CONTRIBUTOR_GUIDE.md). For full guidelines, see [Contributing](CONTRIBUTING.md) and the [developer documentation](docs/development/CONTRIBUTING.md). ## Running Locally diff --git a/README.md b/README.md index 482f6703..38c3650a 100644 --- a/README.md +++ b/README.md @@ -76,7 +76,7 @@ TimeTracker is built with modern, reliable technologies: ## 🖥️ UI overview -The web app uses a **single main layout** with a sidebar and top header. Content is centered with a max width for readability. **Getting around:** **Dashboard** — overview, today’s stats, and the main **Timer** widget (start/stop, quick start, repeat last). **Timer** and **Time entries** are first-class in the sidebar for fast access. **Time entries** is the place to filter, review, and export all logged time. **Reports**, **Projects**, **Finance**, and **Settings** are available from the sidebar and navigation. For design and component conventions, see [UI Guidelines](docs/UI_GUIDELINES.md). +The web app uses a **single main layout** with a sidebar and top header. Content is centered with a max width for readability. **Getting around:** **Dashboard** — overview, today’s stats, and the main **Timer** widget (start/stop, quick start, repeat last). **Timer** and **Time entries** are first-class in the sidebar for fast access. **Time entries** is the place to filter, review, and export all logged time. **Reports** (time, project, finance) are available from the sidebar (top-level **Reports** link or **Finance & Expenses → Reports** for Report Builder, Saved Views, Scheduled Reports), and from the bottom bar on mobile. **Projects**, **Finance**, and **Settings** are available from the sidebar and navigation. For design and component conventions, see [UI Guidelines](docs/UI_GUIDELINES.md). --- @@ -86,20 +86,12 @@ TimeTracker has been continuously enhanced with powerful new features! Here's wh > **📋 For complete release history, see [CHANGELOG.md](CHANGELOG.md)** -**Current version** is defined in `setup.py` (single source of truth). See [CHANGELOG.md](CHANGELOG.md) for release history. +**Current version** is defined in `setup.py` (single source of truth). See [CHANGELOG.md](CHANGELOG.md) for versioned release history. - 📱 **Native Mobile & Desktop Apps** — Flutter mobile app (iOS/Android) and Electron desktop app with time tracking, offline support, and API integration ([Build Guide](BUILD.md), [Docs](docs/mobile-desktop-apps/README.md)) - 📋 **Project Analysis & Documentation** — Comprehensive project analysis and documentation updates - 🔧 **Version Consistency** — Fixed version inconsistencies across documentation files -**Previous Releases:** -- **v4.14.0** (January 2025) — Documentation and technology stack updates -- **v4.6.0** (December 2025) — Comprehensive Issue/Bug Tracking System - -**Recent Releases:** -- **v4.5.1** — Performance optimizations and version management improvements -- **v4.5.0** — Advanced Report Builder, quick task creation, Kanban enhancements, and PWA improvements -- **v4.4.1** — Dashboard cache fixes and custom reports enhancements -- **v4.4.0** — Project custom fields, file attachments, and salesman-based report splitting +See [CHANGELOG.md](CHANGELOG.md) for all release notes and version history. ### 🎯 **Major Feature Additions** @@ -437,7 +429,7 @@ docker-compose up -d # Click "Advanced" → "Proceed to localhost" to continue ``` -**First login creates the admin account** — just enter your username! +**First login creates the admin account** — just enter your username! For setup problems, see [INSTALLATION.md](INSTALLATION.md). **📖 See the complete setup guide:** [`docs/admin/configuration/DOCKER_COMPOSE_SETUP.md`](docs/admin/configuration/DOCKER_COMPOSE_SETUP.md) diff --git a/docs/ARCHITECTURE_AUDIT.md b/docs/ARCHITECTURE_AUDIT.md new file mode 100644 index 00000000..cccfbd9e --- /dev/null +++ b/docs/ARCHITECTURE_AUDIT.md @@ -0,0 +1,64 @@ +# Architecture Audit + +This document captures a concise architecture audit of the TimeTracker repository (Flask app with server-rendered templates, REST API, and desktop/mobile clients). It is used to drive incremental refactors toward thin routes, reusable services, and a predictable repository layer. + +--- + +## Current strengths + +- **Clear intended layering**: Routes → services → repositories/models is documented in [ARCHITECTURE.md](../ARCHITECTURE.md) and [Architecture Migration Guide](implementation-notes/ARCHITECTURE_MIGRATION_GUIDE.md). +- **Existing repository layer**: `app/repositories/` provides `BaseRepository` and dedicated repos for TimeEntry, Project, Task, Client, Invoice, Expense, Payment, User, Comment with sensible methods (e.g. `TimeEntryRepository.get_active_timer`, `get_by_date_range`, `get_total_duration`). +- **Central API response helpers**: `app/utils/api_responses.py` defines `success_response`, `error_response`, `validation_error_response`, `paginated_response`, etc.; error handlers in `app/utils/error_handlers.py` use them for JSON/API. +- **Blueprint registry**: Single registration point in `app/blueprint_registry.py` keeps app init clean. +- **Refactor examples**: The migration guide gives a clear "after" pattern. (Historical note: previously unregistered modules `timer_refactored.py`, `projects_refactored_example.py`, `invoices_refactored.py` have been merged or removed.) +- **Validation and schemas**: Marshmallow used for time-entry API v1; `app/utils/validation.py` and `app/utils/time_entry_validation.py` exist; `app/schemas/` has schemas for several resources (underused in routes). + +--- + +## Main architectural risks + +1. **Business logic in routes**: Heavy logic in `app/routes/reports.py` (comparison, project_report, unpaid_hours), `app/routes/invoices.py` (many direct queries), `app/routes/recurring_invoices.py`, `app/routes/expenses.py`, `app/routes/deals.py`, `app/routes/gantt.py`, `app/routes/comments.py`, `app/routes/client_notes.py`, `app/routes/audit_logs.py`, and others. +2. **Duplicated query patterns**: "User's distinct project IDs" from TimeEntry appears in budget_alerts, reports, data_export, gantt, timer, api with no shared repo method. Same for "user project IDs + accessible client IDs" in issues.py (4 blocks). Sum(TimeEntry.duration_seconds) repeated in analytics, reports, reporting_service, models, and utils despite `TimeEntryRepository.get_total_duration`. +3. **Inconsistent API contract**: Most api_v1 resource routes return resource-keyed payloads (`{"invoices": [...]}`, `{"invoice": {...}}`) and ad-hoc errors instead of the standard envelope (`success`, `data`, `error`, `error_code`) from `api_responses`. +4. **Validation inconsistency**: Only time-entry API v1 uses Marshmallow; other api_v1_* and all web forms use manual checks. Schemas exist for Invoice, Project, Client, Expense, etc. but are not used in corresponding routes. +5. **God files and tight coupling**: `app/routes/api_v1.py`, `app/routes/timer.py`, `app/routes/api.py`, `app/routes/reports.py`, `app/routes/invoices.py` are very large. Large route files mix many endpoints and inline queries. +6. **Logic in models**: `app/models/recurring_invoice.py` `generate_invoice()` does full workflow. `app/models/expense.py`, `app/models/lead.py`, `app/models/issue.py`, `app/models/project.py` contain state transitions and query/aggregation methods that belong in services/repositories. +7. **Template logic**: Budget and status rules in project view/list templates; task counts via `selectattr` in tasks list/my_tasks/kanban; totals and filters in inventory, client portal, expense_categories. Better to precompute in views. +8. **Missing repositories**: No repository for FocusSession, Activity, AuditLog, StockItem/inventory, RecurringInvoice, and others; services and routes use `Model.query` / `db.session` directly. +9. **Unused/refactor-only code**: (Historical: `timer_refactored.py`, `projects_refactored_example.py`, `invoices_refactored.py` are no longer present; refactors were merged or removed.) + +--- + +## Top 10 refactor targets (ranked by impact and risk) + +| # | Target | Impact | Risk | Action | +|---|--------|--------|------|--------| +| 1 | **Centralize "user's distinct project IDs"** | High | Low | Add `TimeEntryRepository.get_distinct_project_ids_for_user(user_id)`; replace 6+ call sites. | +| 2 | **Move RecurringInvoice.generate_invoice to service** | High | Medium | Create `RecurringInvoiceService.generate_invoice(recurring_invoice)`; keep model for state only; add tests. | +| 3 | **Thin reports routes** | High | Medium | Move comparison_view, project_report, unpaid_hours_report (and export) logic into `ReportingService`. | +| 4 | **Standardize API v1 response envelope** | High | Medium | Use `success_response`/`error_response` in api_v1_* routes; document contract. | +| 5 | **Recurring invoices: service + repository** | Medium–High | Medium | Add `RecurringInvoiceRepository` and `RecurringInvoiceService`; move list/create/edit/delete and generate from routes. | +| 6 | **Invoices route thinning (incremental)** | High | High | Move one invoice handler at a time into `InvoiceService` using existing `InvoiceRepository`; add tests per batch. | +| 7 | **Issues: shared "accessible projects/clients" helper** | Medium | Low | Add repository or scope helper; use in all four places in `app/routes/issues.py`. | +| 8 | **API v1 validation and api_responses** | Medium | Low | Use existing schemas + `handle_validation_error` and `success_response`/`error_response` in api_v1_* modules. | +| 9 | **Gantt: extract data and progress to service** | Medium | Low | Add `GanttService` for `get_gantt_data` and progress calculation; route only delegates. | +| 10 | **Precompute task counts and budget in views** | Medium | Low | In tasks/project routes, compute task_counts and budget fields; pass to templates. | + +--- + +## Refactor progress + +| # | Target | Status | +|---|--------|--------| +| 1 | Centralize "user's distinct project IDs" | Done | +| 2 | RecurringInvoiceService.generate_invoice | Done | +| 3 | Thin reports routes | Done | +| 4 | Standardize API v1 response envelope | Done (documented) | +| 5 | Recurring invoices service + repository | Done | +| 6 | Invoices incremental | Done (get_unbilled_data_for_invoice) | +| 7 | Issues accessible IDs helper | Done | +| 8 | API v1 validation and api_responses | Done (api_v1_projects) | +| 9 | Gantt service | Done | +| 10 | Precompute task counts and budget in views | Done | + +*(Update status to Done as refactors are completed.)* diff --git a/docs/DOCS_AUDIT.md b/docs/DOCS_AUDIT.md new file mode 100644 index 00000000..0c349b8b --- /dev/null +++ b/docs/DOCS_AUDIT.md @@ -0,0 +1,73 @@ +# Documentation Audit Summary + +This audit summarizes the state of TimeTracker documentation as of the audit date. Use it to find accurate sources, fix outdated content, and fill gaps. + +--- + +## Accurate (keep; minimal edits only) + +| Doc | Notes | +|-----|--------| +| **README.md** | Tech stack, quick start (Docker HTTPS/HTTP/SQLite), features, doc links correct. Version: states "defined in setup.py"; avoid hardcoding version examples in What's New. | +| **INSTALLATION.md** | Matches actual flow; points to GETTING_STARTED and DOCKER_COMPOSE_SETUP. | +| **DEVELOPMENT.md** | Venv + `flask run`, `docker-compose.local-test.yml`, folder structure, test commands align with repo. | +| **ARCHITECTURE.md** | Module table, data flow, API structure match app/ (blueprint_registry, routes, services, repositories, models). | +| **API.md** (root), **docs/api/REST_API.md** | Accurate overview and auth; REST_API is full reference. | +| **docs/development/SERVICE_LAYER_AND_BASE_CRUD.md** | Accurately describes service/repository pattern and BaseCRUDService. | +| **docs/development/LOCAL_TESTING_WITH_SQLITE.md** | Correct for docker-compose.local-test.yml and scripts. | +| **docs/TESTING_QUICK_REFERENCE.md**, **docs/TESTING_COVERAGE_GUIDE.md** | Align with Makefile targets and pytest markers. | +| **docs/UI_GUIDELINES.md**, **docs/FRONTEND.md** | Match templates (base.html, components/ui.html) and Tailwind. | +| **docs/GETTING_STARTED.md** | Correct: default compose → https://localhost; documents example compose for http://localhost:8080. | +| **requirements-test.txt** | Exists; referenced by Makefile and CI. | + +--- + +## Outdated + +| Item | Location | Fix | +|------|----------|-----| +| Version numbers | README "What's New" / "Current version" | Point to setup.py + CHANGELOG only; remove or generalize hardcoded v4.14.0, v4.6.0. | +| Hardcoded version | docs/development/PROJECT_STRUCTURE.md | Replace "version='4.20.9'" with "version in setup.py (single source of truth)". | +| Hardcoded version | docs/FEATURES_COMPLETE.md | Remove "Version: 4.20.6" or replace with "See setup.py". | +| Docker access URL | docs/development/CONTRIBUTING.md | Default `docker-compose up --build` → https://localhost. For http://localhost:8080 use docker-compose.example.yml or docker-compose.local-test.yml. | +| Compose role | docs/development/PROJECT_STRUCTURE.md | Describe docker-compose.yml as "Default stack (HTTPS via nginx)"; add docker-compose.example.yml (HTTP 8080) and docker-compose.local-test.yml (SQLite). | +| Refactored modules | docs/ARCHITECTURE_AUDIT.md | Note that timer_refactored/projects_refactored/invoices_refactored are historical (merged or removed). | +| Deployment guide label | docs/guides/DEPLOYMENT_GUIDE.md | Content is feature checklist, not "how to deploy". Add note at top pointing to DOCKER_COMPOSE_SETUP and DOCKER_PUBLIC_SETUP. | + +--- + +## Missing + +| Gap | Resolution | +|-----|------------| +| Single contributor onboarding doc | **CONTRIBUTOR_GUIDE.md** — Architecture, local dev, testing, how to add route/service/repository/template, versioning. | +| Versioning for contributors | Short "For contributors" note: app version in setup.py only; desktop/mobile have own config; link BUILD.md and VERSION_MANAGEMENT. | +| Step-by-step "add route/service/repository/template" | Include in CONTRIBUTOR_GUIDE with concrete steps (files, blueprint_registry, tests). | + +--- + +## Duplicated or Overlapping + +| Area | Notes | +|------|--------| +| **Installation** | README Quick Start, INSTALLATION.md, GETTING_STARTED.md, DOCKER_COMPOSE_SETUP all cover install. Keep overlap; add one-line pointers (e.g. "For step-by-step see INSTALLATION.md", "For all env vars see DOCKER_COMPOSE_SETUP"). | +| **Contributing** | Root CONTRIBUTING.md points to docs/development/CONTRIBUTING.md; fix Docker URL in full CONTRIBUTING. | +| **Deployment** | README, INSTALLATION, DOCKER_COMPOSE_SETUP, DOCKER_PUBLIC_SETUP, guides/DEPLOYMENT_GUIDE. Clarify DEPLOYMENT_GUIDE as feature checklist; "how to deploy" = DOCKER_COMPOSE_SETUP / DOCKER_PUBLIC_SETUP. | + +--- + +## Contradictions + +| Issue | Resolution | +|-------|------------| +| **Docker URL** | CONTRIBUTING (full) said http://localhost:8080 after default `docker-compose up --build`. Default compose serves **HTTPS** (https://localhost). Fix: document default = https://localhost; for 8080 use docker-compose.example.yml or docker-compose.local-test.yml. | +| **Compose purpose** | PROJECT_STRUCTURE said "Local development compose" for docker-compose.yml; README uses it for quick start and production. Unify: default = full stack HTTPS; development options = example (HTTP) or local-test (SQLite). | + +--- + +## File reference + +- **Audit doc**: this file — `docs/DOCS_AUDIT.md` +- **Updates**: README.md, docs/development/CONTRIBUTING.md, docs/development/PROJECT_STRUCTURE.md, docs/FEATURES_COMPLETE.md, docs/ARCHITECTURE_AUDIT.md, docs/README.md, docs/guides/DEPLOYMENT_GUIDE.md +- **New**: docs/development/CONTRIBUTOR_GUIDE.md +- **Cross-links**: CONTRIBUTING.md (root), docs/README.md, DEVELOPMENT.md diff --git a/docs/FEATURES_COMPLETE.md b/docs/FEATURES_COMPLETE.md index 1750ee81..9afd7861 100644 --- a/docs/FEATURES_COMPLETE.md +++ b/docs/FEATURES_COMPLETE.md @@ -1,8 +1,10 @@ # TimeTracker - Complete Features Documentation -**Version:** 4.20.6 (see `setup.py` for single source of truth) +**Version:** See `setup.py` for current version (single source of truth). **Last Updated:** 2025-02-20 +**Navigation:** Many features are optional (see Admin → Module Management). Reports are available from the top-level **Reports** sidebar link (or **Finance & Expenses → Reports** for Report Builder, Saved Views, Scheduled Reports). Time entries export is under **Time entries** (overview page). + --- ## Table of Contents diff --git a/docs/FRONTEND.md b/docs/FRONTEND.md new file mode 100644 index 00000000..833315e8 --- /dev/null +++ b/docs/FRONTEND.md @@ -0,0 +1,70 @@ +# TimeTracker Frontend Guide + +This document describes the main app frontend stack, component usage, and conventions. It does **not** cover the client portal or kiosk bases. + +## Stack + +- **Templates**: Jinja2 (Flask) +- **Styles**: Tailwind CSS (built from `app/static/src/input.css` → `app/static/dist/output.css`) +- **Design tokens**: CSS variables and Tailwind theme in `app/static/src/input.css` and `tailwind.config.js` +- **No React/Vue**: The app remains server-rendered with Jinja; use existing macros and minimal JS for behavior. + +References: [UI_IMPROVEMENTS_SUMMARY.md](implementation-notes/UI_IMPROVEMENTS_SUMMARY.md), [STYLING_CONSISTENCY_SUMMARY.md](implementation-notes/STYLING_CONSISTENCY_SUMMARY.md). + +## Base template and blocks + +- **Main app**: `app/templates/base.html` — provides ``, head (meta, CSS, scripts), skip link, sidebar, header, `
`, footer, and mobile bottom nav. +- **Blocks**: `title`, `content`, `extra_css`, `scripts_extra`, `head_extra`, etc. Page templates extend `base.html` and override these blocks. + +## Component usage + +**Primary source**: `app/templates/components/ui.html` (and `app/templates/components/cards.html` where used). Prefer these over legacy `_components.html`. + +### When to use which + +| Need | Macro / component | Import from | +|------|-------------------|-------------| +| Page title + subtitle + optional breadcrumbs and actions | `page_header(icon_class, title_text, subtitle_text=None, actions_html=None, breadcrumbs=None)` | `components/ui.html` | +| Breadcrumbs only | `breadcrumb_nav(items)` | `components/ui.html` | +| Summary / stat block | `stat_card(title, value, icon_class, color, trend=None, subtitle=None)` | `components/ui.html` or `components/cards.html` | +| Empty list / no results | `empty_state(...)` or `empty_state_compact(...)` with `type='no-data'` or `type='no-results'` | `components/ui.html` | +| Buttons | Use classes `.btn`, `.btn-primary`, `.btn-secondary`, `.btn-danger`, `.btn-ghost`, `.btn-sm`, `.btn-lg` from design system | `app/static/src/input.css` | +| Modals | `modal(id, title, content_html, footer_html=None, size)` | `components/ui.html` | +| Confirm (destructive) | `confirm_dialog(id, title, message, confirm_text, cancel_text, confirm_class)` | `components/ui.html` | +| Pagination | `pagination_nav(pagination, route_name, url_params=None, aria_label=None)` | `components/ui.html` | +| Forms | `form_group`, `form_select`, `form_textarea`, etc. | `components/ui.html` | + +### Empty states + +- Use **no-data** when the list is empty and no filters are applied (e.g. “No time entries yet”). +- Use **no-results** when filters are applied but nothing matches (e.g. “No time entries match your filters”). +- Prefer the macro over inline empty markup so messaging and CTAs stay consistent. + +## Buttons and forms + +- **Buttons**: Use design-system classes (`.btn`, `.btn-primary`, `.btn-secondary`, `.btn-danger`, `.btn-ghost`) so focus states and colors stay consistent. Avoid ad-hoc `bg-*` / `px-*` for primary actions. +- **Forms**: Use `form_group` and related form macros from `ui.html` so labels, `aria-required`, `aria-invalid`, and error blocks are consistent. Shared validation lives in `form-validation.js` / `form-validation.css`. + +## Modals + +- Use the **modal** or **confirm_dialog** macros from `components/ui.html` for new and refactored flows. +- Custom dialogs must provide: + - `role="dialog"` and `aria-modal="true"` + - `aria-labelledby` (and preferably `aria-describedby`) pointing to the title and description + - Focus trap when open (keep focus inside the dialog until closed). + +## Tables and pagination + +- **List tables**: Prefer `data-table-enhanced` (see `data-tables-enhanced.js` / `.css`) for sortable headers and consistent ARIA where applicable. +- **Responsive**: Use `responsive-cards` and `data-label` on cells for small screens. +- **Pagination**: Use the `pagination_nav` macro when possible; otherwise wrap pagination in a ` +{% endif %} +{% endmacro %} + {# ============================================ STAT CARDS ============================================ #} @@ -441,19 +474,19 @@

{{ _('Status') }}
- {% if percentage >= 100 %} + {% if budget_status is defined and budget_status == 'over' %} {{ _('Over') }} - {% elif percentage >= threshold %} + {% elif budget_status is defined and budget_status == 'critical' %} {{ _('Critical') }} - {% elif percentage >= (threshold * 0.8) %} + {% elif budget_status is defined and budget_status == 'warning' %} {{ _('Warning') }} - {% else %} + {% elif budget_status is defined and budget_status == 'healthy' %} {{ _('Healthy') }} + {% else %} + — {% endif %}
diff --git a/app/templates/reports/index.html b/app/templates/reports/index.html index 16314fe5..28dc1a18 100644 --- a/app/templates/reports/index.html +++ b/app/templates/reports/index.html @@ -15,6 +15,13 @@ actions_html=None ) }} +{% if not is_license_activated and current_user.is_authenticated %} +

+ {{ _('Enjoying TimeTracker?') }} {{ _('You can support development by purchasing a license key.') }} + {{ _('Support the project') }} +

+{% endif %} +
{{ info_card(_("Total Hours"), "%.2f"|format(summary.total_hours), _("All time")) }} {{ info_card(_("Billable Hours"), "%.2f"|format(summary.billable_hours), _("All time")) }} diff --git a/app/templates/reports/time_entries_report.html b/app/templates/reports/time_entries_report.html index 6c87dfa8..1bc6b3ac 100644 --- a/app/templates/reports/time_entries_report.html +++ b/app/templates/reports/time_entries_report.html @@ -138,5 +138,20 @@

{{ _('Time Entries Report') }}

{% endfor %} + {% if pagination and pagination.pages > 1 %} + + {% endif %}
{% endblock %} diff --git a/app/templates/tasks/my_tasks.html b/app/templates/tasks/my_tasks.html index e59ad1cb..052912ec 100644 --- a/app/templates/tasks/my_tasks.html +++ b/app/templates/tasks/my_tasks.html @@ -11,7 +11,7 @@ {% block content %}
- {% from "_components.html" import page_header %} + {% from "components/ui.html" import page_header %}
@@ -38,7 +38,7 @@
{{ _('To Do') }}
-
{{ tasks|selectattr('status', 'equalto', 'todo')|list|length }}
+
{{ task_counts.get('todo', 0) if task_counts is defined and task_counts else (tasks|selectattr('status', 'equalto', 'todo')|list|length) }}
@@ -55,7 +55,7 @@
{{ _('In Progress') }}
-
{{ tasks|selectattr('status', 'equalto', 'in_progress')|list|length }}
+
{{ task_counts.get('in_progress', 0) if task_counts is defined and task_counts else (tasks|selectattr('status', 'equalto', 'in_progress')|list|length) }}
@@ -72,7 +72,7 @@
{{ _('Review') }}
-
{{ tasks|selectattr('status', 'equalto', 'review')|list|length }}
+
{{ task_counts.get('review', 0) if task_counts is defined and task_counts else (tasks|selectattr('status', 'equalto', 'review')|list|length) }}
@@ -89,7 +89,7 @@
{{ _('Completed') }}
-
{{ tasks|selectattr('status', 'equalto', 'done')|list|length }}
+
{{ task_counts.get('done', 0) if task_counts is defined and task_counts else (tasks|selectattr('status', 'equalto', 'done')|list|length) }}
diff --git a/app/templates/timer/_time_entries_list.html b/app/templates/timer/_time_entries_list.html index e496d756..9d4557b2 100644 --- a/app/templates/timer/_time_entries_list.html +++ b/app/templates/timer/_time_entries_list.html @@ -1,3 +1,4 @@ +{% from "components/ui.html" import empty_state_compact %}
@@ -171,20 +172,23 @@ {% endfor %} {% else %} - - -
-
- -
-

{{ _('No time entries found') }}

-

{{ _('Try adjusting your filters or log a new time entry.') }}

- - {{ _('Log Time') }} - -
- - + {% set has_filters = (filters.start_date or filters.end_date or filters.project_id or filters.client_id or filters.paid or filters.billable or (filters.search and filters.search|trim) or (filters.user_id if can_view_all else none) or (filters.client_custom_field and filters.client_custom_field|length > 0)) %} + {% if has_filters %} + {% set clear_filters_url = url_for('timer.time_entries_overview') %} + {% set empty_actions = '' ~ _('Clear filters') ~ '' ~ _('Log Time') ~ '' %} + + + {{ empty_state_compact('fas fa-search', _('No time entries match your filters'), _('Try adjusting your filters or clear them to see all entries. You can also log a new time entry.'), empty_actions, type='no-results') }} + + + {% else %} + {% set log_time_actions = '' ~ _('Log Time') ~ '' %} + + + {{ empty_state_compact('fas fa-clock', _('No time entries yet'), _('Log time to see your entries here. Use the timer on the dashboard or log time manually.'), log_time_actions, type='no-data') }} + + + {% endif %} {% endif %} @@ -192,7 +196,7 @@ {% if pagination.pages > 1 %} -
+
-
+ {% endif %}
diff --git a/app/templates/timer/bulk_entry.html b/app/templates/timer/bulk_entry.html index e0975097..8138a3b8 100644 --- a/app/templates/timer/bulk_entry.html +++ b/app/templates/timer/bulk_entry.html @@ -63,7 +63,7 @@ {% endblock %}
- {% from "_components.html" import page_header %} + {% from "components/ui.html" import page_header %}
{% set actions %} diff --git a/app/templates/timer/time_entries_overview.html b/app/templates/timer/time_entries_overview.html index cd96f93f..5d71c35a 100644 --- a/app/templates/timer/time_entries_overview.html +++ b/app/templates/timer/time_entries_overview.html @@ -30,31 +30,33 @@

{{ _('Filters') }}

- {{ _('Exports use current filters') }} - - - {{ _('Export CSV') }} + {{ _('Export CSV') }} - {{ _('Export PDF') }} + {{ _('Export PDF') }} -
@@ -128,8 +130,8 @@

{{ _('Filters') }}

{% endfor %} {% endif %}
-
@@ -240,9 +242,11 @@

{{ _('Mark Selected Entries as Paid/Unpai function toggleFilterVisibility() { const filterBody = document.getElementById('filterBody'); const icon = document.getElementById('filterToggleIcon'); + const toggleBtn = document.getElementById('toggleFilters'); filterBody.classList.toggle('hidden'); icon.classList.toggle('fa-chevron-up'); icon.classList.toggle('fa-chevron-down'); + if (toggleBtn) toggleBtn.setAttribute('aria-expanded', filterBody.classList.contains('hidden') ? 'false' : 'true'); } function clearAllFilters() { @@ -668,6 +672,8 @@

{{ _('Mark Selected Entries as Paid/Unpai if (filterBody && filterBody.classList.contains('hidden')) { filterBody.classList.remove('hidden'); if (icon) { icon.classList.remove('fa-chevron-down'); icon.classList.add('fa-chevron-up'); } + var t = document.getElementById('toggleFilters'); + if (t) t.setAttribute('aria-expanded', 'true'); } if (filterTimeout) clearTimeout(filterTimeout); if (searchTimeout) clearTimeout(searchTimeout); diff --git a/app/templates/user/license.html b/app/templates/user/license.html new file mode 100644 index 00000000..adadb75f --- /dev/null +++ b/app/templates/user/license.html @@ -0,0 +1,60 @@ +{% extends "base.html" %} +{% block title %}{{ _('License') }} - {{ _('Settings') }}{% endblock %} + +{% block content %} +
+
+

{{ _('License') }}

+

{{ _('View activation status and enter a license key') }}

+
+ +
+

{{ _('License status') }}

+ {% if is_license_activated %} +

+ + {{ _('Active license') }} +

+

{{ _('Thank you for supporting TimeTracker.') }}

+ {% else %} +

+ + {{ _('Not activated') }} +

+

+ {{ _('Want to support development? You can purchase a license key for €25.') }} +

+ + {{ _('Buy license (€25)') }} + + {% endif %} +
+ + {% if not is_license_activated %} +
+

{{ _('Enter license key') }}

+

{{ _('Paste the license key you received by email after purchase.') }}

+
+ +
+ + +
+ +
+

+ {{ _('Need a key?') }} + {{ _('Purchase a license key') }} +

+
+ {% endif %} + +

+ {{ _('Back to Settings') }} +

+
+{% endblock %} diff --git a/app/templates/user/settings.html b/app/templates/user/settings.html index 95d1ed80..c86c5e4e 100644 --- a/app/templates/user/settings.html +++ b/app/templates/user/settings.html @@ -8,6 +8,21 @@

{{ _('Settings') }}

{{ _('Manage your account settings and preferences') }}

+ {% if not is_license_activated %} +
+

{{ _('TimeTracker is free and open-source. If you enjoy using it, you can support development by purchasing a license key.') }}

+ + {{ _('Buy license (€25)') }} + +
+ {% endif %} + +
+ + {{ _('License') }} + +
+
diff --git a/app/utils/api_auth.py b/app/utils/api_auth.py index 01154490..170525ce 100644 --- a/app/utils/api_auth.py +++ b/app/utils/api_auth.py @@ -133,6 +133,7 @@ def decorated_function(*args, **kwargs): { "error": "Authentication required", "message": "API token must be provided in Authorization header or X-API-Key header", + "error_code": "unauthorized", } ), 401, @@ -144,7 +145,11 @@ def decorated_function(*args, **kwargs): if not user or not api_token: message = error_msg or "The provided API token is invalid or expired" return ( - jsonify({"error": "Invalid token", "message": message}), + jsonify({ + "error": "Invalid token", + "message": message, + "error_code": "unauthorized", + }), 401, ) @@ -155,6 +160,7 @@ def decorated_function(*args, **kwargs): { "error": "Insufficient permissions", "message": f'This endpoint requires the "{required_scope}" scope', + "error_code": "forbidden", "required_scope": required_scope, "available_scopes": api_token.scopes.split(",") if api_token.scopes else [], } diff --git a/app/utils/api_responses.py b/app/utils/api_responses.py index 855f3e0d..80aea133 100644 --- a/app/utils/api_responses.py +++ b/app/utils/api_responses.py @@ -59,7 +59,13 @@ def error_response( Returns: Flask JSON response """ - response = {"success": False, "error": error_code or "error", "message": message} + # error = user-facing message (backward compat); error_code = machine-readable + response = { + "success": False, + "error": message, + "message": message, + "error_code": error_code or "error", + } if errors: response["errors"] = errors @@ -191,7 +197,7 @@ def created_response(data: Any, message: Optional[str] = None, location: Optiona Returns: Flask JSON response """ - response_data = {"data": data} + response_data = {"success": True, "data": data} if message: response_data["message"] = message diff --git a/app/utils/cache.py b/app/utils/cache.py index 7416aeaf..4d806b8b 100644 --- a/app/utils/cache.py +++ b/app/utils/cache.py @@ -252,6 +252,16 @@ def invalidate_cache(pattern: str) -> None: cache.clear() # For now, just clear all (can be improved) +def invalidate_dashboard_for_user(user_id: int) -> None: + """Invalidate all dashboard-related cache keys for a user (stats, chart, legacy).""" + cache = get_cache() + for key in (f"dashboard:{user_id}", f"dashboard:stats:{user_id}", f"dashboard:chart:{user_id}"): + try: + cache.delete(key) + except Exception: + pass + + def invalidate_pattern(pattern: str) -> None: """ Invalidate cache entries matching a pattern. diff --git a/app/utils/context_processors.py b/app/utils/context_processors.py index 8aa66383..f36b9cc4 100644 --- a/app/utils/context_processors.py +++ b/app/utils/context_processors.py @@ -2,6 +2,7 @@ from flask_babel import get_locale from flask_login import current_user from app.models import Settings +from app.utils.license_utils import is_license_activated from app.utils.timezone import ( get_timezone_offset_for_timezone, get_resolved_date_format_key, @@ -37,6 +38,7 @@ def inject_settings(): "resolved_date_format_key": resolved_date, "resolved_time_format_key": resolved_time, "resolved_week_start_day": resolved_week_start, + "is_license_activated": is_license_activated(settings), } except Exception as e: # Log the error but continue with defaults @@ -66,6 +68,7 @@ def inject_settings(): "resolved_date_format_key": resolved_date, "resolved_time_format_key": resolved_time, "resolved_week_start_day": resolved_week_start, + "is_license_activated": False, } @app.context_processor diff --git a/app/utils/data_export.py b/app/utils/data_export.py index d8291519..914c59e6 100644 --- a/app/utils/data_export.py +++ b/app/utils/data_export.py @@ -10,6 +10,7 @@ from zipfile import ZipFile from flask import current_app from app import db +from app.repositories import TimeEntryRepository from app.models import ( User, Project, @@ -293,9 +294,8 @@ def _export_time_entries(user): def _export_user_projects(user): """Export projects user has worked on""" - # Get unique projects from time entries - project_ids = db.session.query(TimeEntry.project_id).filter_by(user_id=user.id).distinct().all() - project_ids = [pid[0] for pid in project_ids] + time_entry_repo = TimeEntryRepository() + project_ids = time_entry_repo.get_distinct_project_ids_for_user(user.id) projects = Project.query.filter(Project.id.in_(project_ids)).all() return [_project_to_dict(p) for p in projects] diff --git a/app/utils/license_utils.py b/app/utils/license_utils.py new file mode 100644 index 00000000..b35d2ada --- /dev/null +++ b/app/utils/license_utils.py @@ -0,0 +1,12 @@ +""" +Optional support/license visibility helpers. + +Instance-level "license activated" state is represented by Settings.donate_ui_hidden +(set when a user verifies the donate-hide / license key). This is non-blocking +monetization awareness only—no paywall or feature gating. +""" + + +def is_license_activated(settings) -> bool: + """Return True if this instance has an active license (donate/support UI hidden).""" + return bool(getattr(settings, "donate_ui_hidden", False)) diff --git a/app/utils/performance.py b/app/utils/performance.py index 9aaafe35..833bd907 100644 --- a/app/utils/performance.py +++ b/app/utils/performance.py @@ -1,92 +1,60 @@ """ -Performance monitoring utilities. -""" - -from typing import Callable, Any -from functools import wraps -import time -from flask import current_app, g - +Optional performance instrumentation: slow-request logging and query-count profiling. -def measure_time(func: Callable) -> Callable: - """ - Decorator to measure function execution time. - - Usage: - @measure_time - def slow_function(): - # Code - """ +Enable via config: +- PERF_LOG_SLOW_REQUESTS_MS: log when request duration exceeds this many ms (0 = disabled) +- PERF_QUERY_PROFILE: when true, track DB query count per request and include in slow-request logs +""" - @wraps(func) - def wrapper(*args, **kwargs): - start_time = time.time() - try: - result = func(*args, **kwargs) - return result - finally: - elapsed = time.time() - start_time - current_app.logger.debug(f"{func.__name__} took {elapsed:.4f} seconds") - # Store in request context if available - if hasattr(g, "performance_metrics"): - g.performance_metrics[func.__name__] = elapsed - else: - g.performance_metrics = {func.__name__: elapsed} +import logging +from flask import g, request +from sqlalchemy import event +from sqlalchemy.engine import Engine - return wrapper +logger = logging.getLogger("timetracker.perf") -def log_slow_queries(threshold: float = 1.0): +def init_performance_logging(app): """ - Decorator to log slow database queries. - - Args: - threshold: Time threshold in seconds + Register slow-request logging and optional query-count profiling. + No overhead when PERF_LOG_SLOW_REQUESTS_MS is 0 and PERF_QUERY_PROFILE is False. """ + slow_ms = app.config.get("PERF_LOG_SLOW_REQUESTS_MS", 0) or 0 + query_profile = app.config.get("PERF_QUERY_PROFILE", False) - def decorator(func: Callable) -> Callable: - @wraps(func) - def wrapper(*args, **kwargs): - start_time = time.time() - try: - result = func(*args, **kwargs) - return result - finally: - elapsed = time.time() - start_time - if elapsed > threshold: - current_app.logger.warning( - f"Slow query in {func.__name__}: {elapsed:.4f} seconds " f"(threshold: {threshold}s)" - ) - - return wrapper + if query_profile: - return decorator + @app.before_request + def _perf_set_query_count(): + g._perf_query_count = 0 + @event.listens_for(Engine, "before_cursor_execute") + def _perf_count_query(conn, cursor, statement, parameters, context, executemany): + if hasattr(g, "_perf_query_count"): + g._perf_query_count += 1 -class PerformanceMonitor: - """Context manager for performance monitoring""" - - def __init__(self, operation_name: str): - self.operation_name = operation_name - self.start_time = None - - def __enter__(self): - self.start_time = time.time() - return self - - def __exit__(self, exc_type, exc_val, exc_tb): - elapsed = time.time() - self.start_time - current_app.logger.info(f"Performance: {self.operation_name} took {elapsed:.4f} seconds") - return False - - -def get_performance_metrics() -> dict: - """ - Get performance metrics from request context. - - Returns: - dict with performance metrics - """ - if hasattr(g, "performance_metrics"): - return g.performance_metrics - return {} + @app.after_request + def _perf_log_slow_requests(response): + if slow_ms <= 0: + return response + try: + start = getattr(g, "_start_time", None) + if start is None: + return response + duration_ms = (__import__("time").time() - start) * 1000 + if duration_ms < slow_ms: + return response + query_count = getattr(g, "_perf_query_count", getattr(g, "query_count", None)) + if query_count is not None: + logger.warning( + "slow_request path=%s duration_ms=%.0f status=%s query_count=%s", + request.path, duration_ms, response.status_code, query_count, + ) + else: + logger.warning( + "slow_request path=%s duration_ms=%.0f status=%s", + request.path, duration_ms, response.status_code, + ) + except Exception: + pass + return response diff --git a/app/utils/scope_filter.py b/app/utils/scope_filter.py index 4fbee477..56162784 100644 --- a/app/utils/scope_filter.py +++ b/app/utils/scope_filter.py @@ -1,5 +1,7 @@ """Scope filtering for subcontractor role: restrict data to assigned clients/projects.""" +from typing import Tuple, Set + from flask_login import current_user @@ -71,3 +73,35 @@ def user_can_access_project(user, project_id): return True allowed = user.get_allowed_project_ids() return allowed is not None and project_id in allowed + + +def get_accessible_project_and_client_ids_for_user(user_id: int) -> Tuple[Set[int], Set[int]]: + """ + Return (accessible_project_ids, accessible_client_ids) for issue-style access: + projects the user has time entries for or is assigned to tasks on, and clients of those projects. + Used to filter issues for non-admin users without view_all_issues permission. + """ + from app.repositories import TimeEntryRepository + from app.models import Task, Project + + time_entry_repo = TimeEntryRepository() + user_project_ids = set(time_entry_repo.get_distinct_project_ids_for_user(user_id)) + task_project_rows = ( + Task.query.with_entities(Task.project_id) + .filter_by(assigned_to=user_id) + .filter(Task.project_id.isnot(None)) + .distinct() + .all() + ) + task_project_ids = {r[0] for r in task_project_rows} + all_accessible_project_ids = user_project_ids | task_project_ids + if not all_accessible_project_ids: + return set(), set() + client_rows = ( + Project.query.with_entities(Project.client_id) + .filter(Project.id.in_(all_accessible_project_ids), Project.client_id.isnot(None)) + .distinct() + .all() + ) + accessible_client_ids = {r[0] for r in client_rows} + return all_accessible_project_ids, accessible_client_ids From 548de62dde21e5c5f1e4d6ea5e794dcc3a07717f Mon Sep 17 00:00:00 2001 From: Dries Peeters Date: Sun, 15 Mar 2026 09:37:15 +0100 Subject: [PATCH 3/4] test: extend fixtures and add scope, auth, recurring, reports tests - Extend conftest and factories for API and scope tests - Add test_auth, test_reports_scope, test_timer_scope - Add test_recurring_invoice_service, test_scope_filter - Add test_admin_dashboard_charts, test_api_contract, test_reports_task_report - Update test_invoices, test_project_archiving_models, test_project_costs, test_time_entry_repository, test_utils --- tests/conftest.py | 89 +++++++ tests/factories.py | 49 ++++ tests/test_admin_dashboard_charts.py | 38 +++ tests/test_api_contract.py | 169 +++++++++++++ tests/test_invoices.py | 45 ++-- tests/test_project_archiving_models.py | 38 +-- tests/test_project_costs.py | 41 +-- tests/test_reports_task_report.py | 154 ++++++++++++ .../test_time_entry_repository.py | 21 ++ tests/test_routes/test_auth.py | 45 ++++ tests/test_routes/test_reports_scope.py | 43 ++++ tests/test_routes/test_timer_scope.py | 72 ++++++ .../test_recurring_invoice_service.py | 25 ++ tests/test_utils.py | 22 +- tests/test_utils/test_scope_filter.py | 237 ++++++++++++++++++ 15 files changed, 1018 insertions(+), 70 deletions(-) create mode 100644 tests/test_admin_dashboard_charts.py create mode 100644 tests/test_api_contract.py create mode 100644 tests/test_reports_task_report.py create mode 100644 tests/test_routes/test_auth.py create mode 100644 tests/test_routes/test_reports_scope.py create mode 100644 tests/test_routes/test_timer_scope.py create mode 100644 tests/test_services/test_recurring_invoice_service.py create mode 100644 tests/test_utils/test_scope_filter.py diff --git a/tests/conftest.py b/tests/conftest.py index 76a9023b..074f0eb1 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -44,6 +44,7 @@ TimeEntryTemplate, Activity, UserFavoriteProject, + UserClient, ClientNote, WeeklyTimeGoal, Expense, @@ -190,6 +191,7 @@ def app(app_config): TimeEntryTemplate, Activity, UserFavoriteProject, + UserClient, ClientNote, WeeklyTimeGoal, Expense, @@ -815,6 +817,93 @@ def auth_headers(user): return {} +# Default scopes for API token (full access to common resources) +DEFAULT_API_TOKEN_SCOPES = ( + "read:projects,write:projects,read:time_entries,write:time_entries," + "read:tasks,write:tasks,read:clients,write:clients,read:reports,read:users" +) + + +@pytest.fixture +def api_token(app, user): + """Create an API token for the given user with default full scopes. Returns (token_model, plain_token).""" + with app.app_context(): + token, plain_token = ApiToken.create_token( + user_id=user.id, + name="Test API Token", + scopes=DEFAULT_API_TOKEN_SCOPES, + expires_days=30, + ) + db.session.add(token) + db.session.commit() + return token, plain_token + + +@pytest.fixture +def client_with_token(app, api_token): + """Test client with Authorization: Bearer . Use for API tests.""" + token_model, plain_token = api_token + test_client = app.test_client() + test_client.environ_base["HTTP_AUTHORIZATION"] = f"Bearer {plain_token}" + return test_client + + +@pytest.fixture +def scope_restricted_user(app, test_client): + """ + User with subcontractor role and one assigned client (scope-restricted). + Use with project fixture that uses this client so user_can_access_project is True for that project only. + """ + role = Role.query.filter_by(name="subcontractor").first() + if not role: + role = Role(name="subcontractor", description="Restricted to assigned clients") + db.session.add(role) + db.session.flush() + + sub_user = User( + username="scope_restricted_user", + email="sub@example.com", + role="user", + ) + sub_user.is_active = True + sub_user.set_password("password123") + db.session.add(sub_user) + db.session.flush() + + if role not in sub_user.roles: + sub_user.roles.append(role) + db.session.flush() + + # Assign the single test client so this user can only access that client and its projects + uc = UserClient(user_id=sub_user.id, client_id=test_client.id) + db.session.add(uc) + db.session.commit() + db.session.refresh(sub_user) + # Force load relationships so they are available when user is used in tests + _ = list(sub_user.roles) + _ = list(sub_user.assigned_clients.all()) + return sub_user + + +@pytest.fixture +def scope_restricted_authenticated_client(client, scope_restricted_user): + """Test client logged in as scope_restricted_user (subcontractor with one assigned client).""" + login_data = {"username": scope_restricted_user.username, "password": "password123"} + headers = {} + try: + from flask import current_app + + if current_app.config.get("WTF_CSRF_ENABLED"): + resp = client.get("/auth/csrf-token") + token = (resp.get_json() or {}).get("csrf_token", "") if resp.is_json else "" + login_data["csrf_token"] = token + headers["X-CSRFToken"] = token + except Exception: + pass + client.post("/login", data=login_data, headers=headers or None, follow_redirects=True) + return client + + @pytest.fixture def regular_user(user): """Alias for user fixture (regular non-admin user).""" diff --git a/tests/factories.py b/tests/factories.py index ef14eba4..0167884a 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -20,6 +20,8 @@ Task, Payment, ExpenseCategory, + ApiToken, + RecurringInvoice, ) @@ -174,3 +176,50 @@ class Meta: requires_receipt = True requires_approval = True is_active = True + + +class ApiTokenFactory: + """ + Factory for API tokens. ApiToken must be created via create_token() for correct hashing. + Returns (token_model, plain_token). Usage: + token, plain = ApiTokenFactory.create(user_id=user.id, scopes="read:projects") + """ + + @classmethod + def create( + cls, + user_id, + name="Test API Token", + description="", + scopes="read:projects,write:projects,read:time_entries,write:time_entries", + expires_days=30, + ): + token, plain = ApiToken.create_token( + user_id=user_id, + name=name, + description=description, + scopes=scopes, + expires_days=expires_days, + ) + db.session.add(token) + db.session.flush() + return token, plain + + +class RecurringInvoiceFactory(_SessionFactory): + class Meta: + model = RecurringInvoice + + name = factory.Sequence(lambda n: f"Recurring {n}") + project_fk = factory.SubFactory(ProjectFactory) + project_id = factory.SelfAttribute("project_fk.id") + client_id = factory.SelfAttribute("project_fk.client_id") + client_name = factory.LazyAttribute(lambda o: f"Client {o.client_id}") + frequency = "monthly" + interval = 1 + next_run_date = factory.LazyFunction(lambda: _dt.date.today() + _dt.timedelta(days=1)) + due_date_days = 30 + tax_rate = Decimal("20.00") + currency_code = "EUR" + is_active = True + created_by = factory.LazyAttribute(lambda _: UserFactory().id) diff --git a/tests/test_admin_dashboard_charts.py b/tests/test_admin_dashboard_charts.py new file mode 100644 index 00000000..d4fa4b59 --- /dev/null +++ b/tests/test_admin_dashboard_charts.py @@ -0,0 +1,38 @@ +""" +Tests for admin dashboard chart data (optimized GROUP BY queries). +""" + +from datetime import datetime, timedelta + + +def test_admin_dashboard_returns_chart_data_with_30_days(client, app, admin_user): + """Admin dashboard returns 200 and chart data with 30 points for user_activity and time_entries_daily.""" + from app import db + from app.models import TimeEntry, Settings + + with app.app_context(): + # Ensure admin module accessible + settings = Settings.get_settings() + disabled = list(settings.disabled_module_ids or []) + if "admin" in disabled: + settings.disabled_module_ids = [m for m in disabled if m != "admin"] + db.session.add(settings) + db.session.commit() + + with client.session_transaction() as sess: + sess["_user_id"] = str(admin_user.id) + sess["_fresh"] = True + + resp = client.get("/admin", follow_redirects=False) + assert resp.status_code == 200, f"Expected 200, got {resp.status_code} (redirect to login?)" + + data = resp.get_data(as_text=True) + # Admin dashboard shows stats and charts (template uses chart_data.user_activity etc.) + assert "Admin" in data or "Dashboard" in data or "dashboard" in data + # Chart JS uses these canvas/context IDs when chart_data is present + assert "userActivityChart" in data or "timeEntryChart" in data or "projectStatusChart" in data + # 30-day series: page should contain many date strings (YYYY-MM-DD) + import re + date_pattern = r"\d{4}-\d{2}-\d{2}" + dates_in_page = re.findall(date_pattern, data) + assert len(dates_in_page) >= 30, "Chart data should include at least 30 date values (30-day series)" diff --git a/tests/test_api_contract.py b/tests/test_api_contract.py new file mode 100644 index 00000000..690dfc34 --- /dev/null +++ b/tests/test_api_contract.py @@ -0,0 +1,169 @@ +""" +API contract tests: assert standardized response shapes for errors, pagination, and validation. +See docs/api/API_CONSISTENCY_AUDIT.md for the contract. + +Uses app and client from conftest to avoid duplicate DB setup and schema issues. +""" + +import pytest + +pytestmark = [pytest.mark.api, pytest.mark.integration] + +import json +from app import db +from app.models import User, Project, Client, ApiToken + + +@pytest.fixture +def contract_user_id(app): + """Create user for contract tests; return id to avoid detached instance.""" + with app.app_context(): + user = User(username="contractuser", email="contract@example.com") + user.set_password("password") + user.is_active = True + db.session.add(user) + db.session.commit() + return int(user.id) + + +@pytest.fixture +def api_token_read_only(app, contract_user_id): + """Token with read:projects only (no write).""" + with app.app_context(): + token, plain = ApiToken.create_token( + user_id=contract_user_id, name="Read only", scopes="read:projects" + ) + db.session.add(token) + db.session.commit() + return plain + + +@pytest.fixture +def api_token_time_entries_only(app, contract_user_id): + """Token with read:time_entries, write:time_entries only (no projects scope).""" + with app.app_context(): + token, plain = ApiToken.create_token( + user_id=contract_user_id, + name="Time entries only", + scopes="read:time_entries,write:time_entries", + ) + db.session.add(token) + db.session.commit() + return plain + + +@pytest.fixture +def api_token_full(app, contract_user_id): + """Token with read/write for projects and time_entries.""" + with app.app_context(): + token, plain = ApiToken.create_token( + user_id=contract_user_id, + name="Full", + scopes="read:projects,write:projects,read:time_entries,write:time_entries", + ) + db.session.add(token) + db.session.commit() + return plain + + +@pytest.fixture +def test_project(app, contract_user_id): + with app.app_context(): + client_model = Client(name="Contract Client", email="c@example.com") + db.session.add(client_model) + db.session.commit() + project = Project( + name="Contract Project", + status="active", + client_id=client_model.id, + ) + db.session.add(project) + db.session.commit() + return project + + +class TestErrorResponseContract: + """Error responses MUST include error, message, and optional error_code.""" + + def test_401_includes_error_message_and_error_code(self, client): + response = client.get("/api/v1/projects") + assert response.status_code == 401 + data = json.loads(response.data) + assert "error" in data + assert "message" in data + assert data.get("error_code") == "unauthorized" + + def test_403_scope_includes_error_message_and_error_code(self, client, api_token_read_only): + headers = {"Authorization": f"Bearer {api_token_read_only}"} + response = client.post( + "/api/v1/projects", + json={"name": "New"}, + headers=headers, + content_type="application/json", + ) + assert response.status_code == 403 + data = json.loads(response.data) + assert "error" in data + assert "message" in data + assert data.get("error_code") == "forbidden" + assert "required_scope" in data + assert "available_scopes" in data + + def test_403_get_projects_with_time_entries_only_token(self, client, api_token_time_entries_only): + """GET /api/v1/projects with token that has only read:time_entries returns 403 (requires read:projects).""" + headers = {"Authorization": f"Bearer {api_token_time_entries_only}"} + response = client.get("/api/v1/projects", headers=headers) + assert response.status_code == 403 + data = json.loads(response.data) + assert data.get("error_code") == "forbidden" + assert "required_scope" in data + assert "available_scopes" in data + assert "read:projects" in str(data.get("required_scope", "")) + + def test_400_validation_includes_error_code_and_errors(self, client, api_token_full): + """Creating a project without name returns validation_error and errors dict.""" + headers = {"Authorization": f"Bearer {api_token_full}"} + response = client.post( + "/api/v1/projects", + json={}, + headers=headers, + content_type="application/json", + ) + assert response.status_code == 400 + data = json.loads(response.data) + assert "error" in data + assert "message" in data + assert data.get("error_code") == "validation_error" + assert "errors" in data + assert "name" in data["errors"] + + def test_404_not_found_includes_error_code(self, client, api_token_full): + headers = {"Authorization": f"Bearer {api_token_full}"} + response = client.get("/api/v1/projects/999999", headers=headers) + assert response.status_code == 404 + data = json.loads(response.data) + assert "error" in data + assert "message" in data + assert data.get("error_code") == "not_found" + + +class TestPaginationContract: + """List responses MUST use resource-named key + pagination with standard keys.""" + + def test_projects_list_has_projects_and_pagination(self, client, api_token_full, test_project): + headers = {"Authorization": f"Bearer {api_token_full}"} + response = client.get("/api/v1/projects", headers=headers) + assert response.status_code == 200 + data = json.loads(response.data) + assert "projects" in data + assert "pagination" in data + pag = data["pagination"] + for key in ("page", "per_page", "total", "pages", "has_next", "has_prev", "next_page", "prev_page"): + assert key in pag, f"pagination must include {key}" + + def test_pagination_default_per_page(self, client, api_token_full): + headers = {"Authorization": f"Bearer {api_token_full}"} + response = client.get("/api/v1/projects", headers=headers) + assert response.status_code == 200 + data = json.loads(response.data) + assert data["pagination"]["per_page"] == 50 diff --git a/tests/test_invoices.py b/tests/test_invoices.py index 20103d08..201a73a4 100644 --- a/tests/test_invoices.py +++ b/tests/test_invoices.py @@ -765,32 +765,33 @@ def test_invoice_sorted_payments_property(app, sample_invoice, sample_user): @pytest.mark.invoices def test_invoice_sorted_payments_with_same_date(app, sample_invoice, sample_user): """Test that sorted_payments handles payments with same payment_date correctly.""" + from unittest.mock import patch from app.models.payments import Payment - import time - # Create payments with the same payment_date but different created_at times - same_date = date.today() + # Deterministic created_at ordering without time.sleep (freezegun incompatible with Py3.14) + t0 = datetime(2024, 1, 1, 10, 0, 0) + t1 = datetime(2024, 1, 1, 10, 0, 1) + with patch("app.models.payments.datetime") as mock_dt: + mock_dt.utcnow.side_effect = [t0, t0, t1, t1] # created_at, updated_at per payment + same_date = date.today() - payment1 = PaymentFactory( - invoice_id=sample_invoice.id, - amount=Decimal("100.00"), - payment_date=same_date, - method="bank_transfer", - received_by=sample_user.id, - ) - db.session.commit() - - # Small delay to ensure different created_at - time.sleep(0.01) + payment1 = PaymentFactory( + invoice_id=sample_invoice.id, + amount=Decimal("100.00"), + payment_date=same_date, + method="bank_transfer", + received_by=sample_user.id, + ) + db.session.commit() - payment2 = PaymentFactory( - invoice_id=sample_invoice.id, - amount=Decimal("200.00"), - payment_date=same_date, - method="credit_card", - received_by=sample_user.id, - ) - db.session.commit() + payment2 = PaymentFactory( + invoice_id=sample_invoice.id, + amount=Decimal("200.00"), + payment_date=same_date, + method="credit_card", + received_by=sample_user.id, + ) + db.session.commit() # Get sorted payments sorted_payments = sample_invoice.sorted_payments diff --git a/tests/test_project_archiving_models.py b/tests/test_project_archiving_models.py index 6d56c678..9987dc20 100644 --- a/tests/test_project_archiving_models.py +++ b/tests/test_project_archiving_models.py @@ -144,34 +144,34 @@ def test_archive_without_parameters(self, app, project): def test_archive_updates_updated_at(self, app, project): """Test that archive() updates the updated_at timestamp""" + from unittest.mock import patch from app import db + from datetime import datetime as dt original_updated_at = project.updated_at - # Wait a tiny bit to ensure timestamp difference - import time - - time.sleep(0.01) - - project.archive() + t1 = dt(2030, 1, 1, 10, 0, 1) # Future so clearly > original_updated_at + with patch("app.models.project.datetime") as mock_dt: + mock_dt.utcnow.return_value = t1 + project.archive() db.session.commit() assert project.updated_at > original_updated_at def test_archive_can_be_called_multiple_times(self, app, project, admin_user): """Test that archive() can be called multiple times (re-archiving)""" + from unittest.mock import patch from app import db + from datetime import datetime as dt - # First archive - project.archive(user_id=admin_user.id, reason="First time") + with patch("app.models.project.datetime") as mock_dt: + mock_dt.utcnow.return_value = dt(2024, 1, 1, 10, 0, 0) + project.archive(user_id=admin_user.id, reason="First time") db.session.commit() first_archived_at = project.archived_at - import time - - time.sleep(0.01) - - # Second archive with different reason - project.archive(user_id=admin_user.id, reason="Second time") + with patch("app.models.project.datetime") as mock_dt2: + mock_dt2.utcnow.return_value = dt(2024, 1, 1, 10, 0, 1) + project.archive(user_id=admin_user.id, reason="Second time") db.session.commit() assert project.status == "archived" @@ -236,17 +236,17 @@ def test_unarchive_clears_archived_reason(self, app, project, admin_user): def test_unarchive_updates_updated_at(self, app, project, admin_user): """Test that unarchive() updates the updated_at timestamp""" + from unittest.mock import patch from app import db + from datetime import datetime as dt project.archive(user_id=admin_user.id, reason="Test") db.session.commit() original_updated_at = project.updated_at - import time - - time.sleep(0.01) - - project.unarchive() + with patch("app.models.project.datetime") as mock_dt: + mock_dt.utcnow.return_value = dt(2030, 1, 1, 10, 0, 1) # Future so updated_at increases + project.unarchive() db.session.commit() assert project.updated_at > original_updated_at diff --git a/tests/test_project_costs.py b/tests/test_project_costs.py index ef8e9da4..3d8fcb76 100644 --- a/tests/test_project_costs.py +++ b/tests/test_project_costs.py @@ -16,6 +16,8 @@ from app.models import User, Project, Client, Invoice, ProjectCost from factories import InvoiceFactory +pytestmark = [pytest.mark.unit, pytest.mark.models] + @pytest.fixture def app(): @@ -291,31 +293,36 @@ def test_is_invoiced_property(self, app, test_project, test_user, test_invoice): def test_mark_as_invoiced(self, app, test_project, test_user, test_invoice): """Test marking a cost as invoiced.""" + from unittest.mock import patch + from datetime import datetime as dt + + t0 = dt(2024, 1, 1, 10, 0, 0) + t1 = dt(2024, 1, 1, 10, 0, 1) with app.app_context(): - cost = ProjectCost( - project_id=test_project, - user_id=test_user, - description="Test cost", - category="materials", - amount=Decimal("75.00"), - cost_date=date.today(), - ) - db.session.add(cost) - db.session.commit() + with patch("app.models.project_cost.datetime") as mock_dt: + mock_dt.utcnow.side_effect = [t0, t1] # initial updated_at, then in mark_as_invoiced + cost = ProjectCost( + project_id=test_project, + user_id=test_user, + description="Test cost", + category="materials", + amount=Decimal("75.00"), + cost_date=date.today(), + ) + db.session.add(cost) + db.session.commit() original_updated_at = cost.updated_at - # Small delay to ensure timestamp changes - import time - - time.sleep(0.01) - - cost.mark_as_invoiced(test_invoice) + with patch("app.models.project_cost.datetime") as mock_dt2: + mock_dt2.utcnow.return_value = t1 + cost.mark_as_invoiced(test_invoice) db.session.commit() assert cost.invoiced is True assert cost.invoice_id == test_invoice - # Note: updated_at might not change in all databases + # updated_at should be later after mark_as_invoiced + assert cost.updated_at > original_updated_at def test_unmark_as_invoiced(self, app, test_project, test_user, test_invoice): """Test unmarking a cost as invoiced.""" diff --git a/tests/test_reports_task_report.py b/tests/test_reports_task_report.py new file mode 100644 index 00000000..7fe5d587 --- /dev/null +++ b/tests/test_reports_task_report.py @@ -0,0 +1,154 @@ +""" +Tests for the task report view and Excel export (behavior and N+1 optimization). +""" + +from datetime import datetime, timedelta + + +def test_task_report_returns_correct_hours_and_entries(client, app, admin_user, user, project, task): + """Task report shows tasks with time entries in range and correct hours/entry count.""" + from app import db + from app.models import TimeEntry, Settings + + with app.app_context(): + settings = Settings.get_settings() + disabled = list(settings.disabled_module_ids or []) + if "reports" in disabled: + settings.disabled_module_ids = [m for m in disabled if m != "reports"] + db.session.add(settings) + db.session.commit() + + start_dt = datetime.utcnow() - timedelta(days=5) + end_dt = datetime.utcnow() - timedelta(days=1) + e1 = TimeEntry( + user_id=user.id, + project_id=project.id, + task_id=task.id, + start_time=start_dt, + end_time=start_dt + timedelta(hours=2), + duration_seconds=7200, + notes="Entry one", + billable=True, + source="manual", + ) + e2 = TimeEntry( + user_id=user.id, + project_id=project.id, + task_id=task.id, + start_time=start_dt + timedelta(hours=3), + end_time=start_dt + timedelta(hours=5), + duration_seconds=7200, + notes="Entry two", + billable=False, + source="manual", + ) + db.session.add_all([e1, e2]) + db.session.commit() + + with client.session_transaction() as sess: + sess["_user_id"] = str(admin_user.id) + sess["_fresh"] = True + + start_date = (datetime.utcnow() - timedelta(days=7)).strftime("%Y-%m-%d") + end_date = datetime.utcnow().strftime("%Y-%m-%d") + + resp = client.get( + f"/reports/tasks?start_date={start_date}&end_date={end_date}&project_id={project.id}&user_id={user.id}", + follow_redirects=False, + ) + + assert resp.status_code == 200 + data = resp.get_data(as_text=True) + assert task.name in data + # 2h + 2h = 4h total for the task + assert "4.0" in data or "4.00" in data + assert "2" in data # entries_count + + +def test_task_report_excel_export_returns_correct_hours(client, app, admin_user, user, project, task): + """Task report Excel export has correct task and hours.""" + from app import db + from app.models import TimeEntry, Settings + from openpyxl import load_workbook + import io + + with app.app_context(): + settings = Settings.get_settings() + disabled = list(settings.disabled_module_ids or []) + if "reports" in disabled: + settings.disabled_module_ids = [m for m in disabled if m != "reports"] + db.session.add(settings) + db.session.commit() + + start_dt = datetime.utcnow() - timedelta(days=3) + e = TimeEntry( + user_id=user.id, + project_id=project.id, + task_id=task.id, + start_time=start_dt, + end_time=start_dt + timedelta(hours=1, minutes=30), + duration_seconds=5400, + notes="Single entry", + billable=True, + source="manual", + ) + db.session.add(e) + db.session.commit() + + with client.session_transaction() as sess: + sess["_user_id"] = str(admin_user.id) + sess["_fresh"] = True + + start_date = (datetime.utcnow() - timedelta(days=7)).strftime("%Y-%m-%d") + end_date = datetime.utcnow().strftime("%Y-%m-%d") + + resp = client.get( + f"/reports/task/export/excel?start_date={start_date}&end_date={end_date}&project_id={project.id}&user_id={user.id}", + follow_redirects=False, + ) + + assert resp.status_code == 200 + assert "spreadsheetml.sheet" in (resp.headers.get("Content-Type") or "") + + wb = load_workbook(filename=io.BytesIO(resp.data)) + ws = wb.active + rows = [r for r in ws.iter_rows(values_only=True) if any(v not in (None, "") for v in (r or []))] + assert len(rows) >= 2 # header + at least one data row + # Header: Task, Project, Status, Completed At, Hours + assert "Task" in str(rows[0]) + # Data row should have task name and 1.5 hours + data_row = next((r for r in rows[1:] if task.name in str(r)), None) + assert data_row is not None + numbers = [float(x) for x in (data_row or []) if isinstance(x, (int, float))] + assert any(abs(n - 1.5) < 0.01 for n in numbers), f"Expected ~1.5 in {data_row}" + + +def test_time_entry_repository_get_task_aggregates(app, project, task, user): + """Repository get_task_aggregates returns correct (task_id, total_seconds, entry_count).""" + from app import db + from app.models import TimeEntry + from app.repositories import TimeEntryRepository + + with app.app_context(): + start_dt = datetime.utcnow() - timedelta(days=1) + end_dt = datetime.utcnow() + for _ in range(2): + e = TimeEntry( + user_id=user.id, + project_id=project.id, + task_id=task.id, + start_time=start_dt, + end_time=start_dt + timedelta(hours=1), + duration_seconds=3600, + source="manual", + ) + db.session.add(e) + db.session.commit() + + repo = TimeEntryRepository() + agg = repo.get_task_aggregates([task.id], start_dt, end_dt) + assert len(agg) == 1 + tid, total_sec, cnt = agg[0] + assert tid == task.id + assert total_sec == 7200 + assert cnt == 2 diff --git a/tests/test_repositories/test_time_entry_repository.py b/tests/test_repositories/test_time_entry_repository.py index eb82677f..1e38fd1e 100644 --- a/tests/test_repositories/test_time_entry_repository.py +++ b/tests/test_repositories/test_time_entry_repository.py @@ -136,3 +136,24 @@ def test_get_by_date_range(self, repository, db_session, sample_user, sample_pro assert len(entries) == 1 assert entries[0].id == entry1.id + + def test_get_distinct_project_ids_for_user(self, repository, db_session, sample_user, sample_project): + """Test getting distinct project IDs for a user from their time entries""" + # Create entries on one project + for _ in range(2): + repository.create_manual_entry( + user_id=sample_user.id, + project_id=sample_project.id, + start_time=datetime.now() - timedelta(hours=1), + end_time=datetime.now(), + ) + db_session.commit() + + project_ids = repository.get_distinct_project_ids_for_user(sample_user.id) + assert project_ids == [sample_project.id] + + # User with no entries returns empty list + other_user = User(username="otheruser", role="user") + db_session.add(other_user) + db_session.commit() + assert repository.get_distinct_project_ids_for_user(other_user.id) == [] diff --git a/tests/test_routes/test_auth.py b/tests/test_routes/test_auth.py new file mode 100644 index 00000000..d3acc620 --- /dev/null +++ b/tests/test_routes/test_auth.py @@ -0,0 +1,45 @@ +""" +Web auth route tests: login page, success redirect, wrong password. +""" + +import pytest + +pytestmark = [pytest.mark.routes, pytest.mark.integration] + + +def test_login_page_returns_200(client): + """GET /login returns 200 and shows login form.""" + resp = client.get("/login") + assert resp.status_code == 200 + data = resp.get_data(as_text=True) + assert "login" in data.lower() or "username" in data.lower() or "sign" in data.lower() + + +def test_login_success_redirects(client, user): + """POST /login with valid credentials redirects or ends on dashboard.""" + resp = client.post( + "/login", + data={"username": user.username, "password": "password123"}, + follow_redirects=True, + ) + # Either we got a redirect (302/303) and followed to dashboard, or direct 200 dashboard + assert resp.status_code == 200 + data = resp.get_data(as_text=True) + # Should not still be on login form (no "Invalid" or "password" error for success path) + # and should show dashboard or main app content + assert "dashboard" in data.lower() or "welcome" in data.lower() or "timer" in data.lower() or "project" in data.lower() + + +def test_login_wrong_password_returns_200_with_message(client, user): + """POST /login with wrong password returns 200 and error message, no redirect to dashboard.""" + resp = client.post( + "/login", + data={"username": user.username, "password": "wrongpassword"}, + follow_redirects=True, + ) + assert resp.status_code == 200 + data = resp.get_data(as_text=True) + # Should still be on login page (login form or error message present) + assert "login" in data.lower() or "invalid" in data.lower() or "password" in data.lower() or "username" in data.lower() + # Should not be dashboard + assert "dashboard" not in data.lower() or "login" in data.lower() diff --git a/tests/test_routes/test_reports_scope.py b/tests/test_routes/test_reports_scope.py new file mode 100644 index 00000000..6bbb3b42 --- /dev/null +++ b/tests/test_routes/test_reports_scope.py @@ -0,0 +1,43 @@ +""" +Reports scope tests: scope-restricted user only sees allowed projects in report views. +""" + +import pytest + +from app import db + +pytestmark = [pytest.mark.routes, pytest.mark.integration] + + +def test_project_report_lists_only_allowed_projects( + app, scope_restricted_authenticated_client, scope_restricted_user, project, test_client +): + """GET /reports/project as scope-restricted user returns page with only assigned project in project list.""" + with app.app_context(): + from app.models import Client, Project, Settings + + settings = Settings.get_settings() + disabled = list(settings.disabled_module_ids or []) + if "reports" in disabled: + settings.disabled_module_ids = [m for m in disabled if m != "reports"] + db.session.add(settings) + db.session.commit() + + other_client = Client(name="Other Corp", email="other@example.com") + other_client.status = "active" + db.session.add(other_client) + db.session.commit() + other_project = Project(name="Other Project", client_id=other_client.id, status="active") + db.session.add(other_project) + db.session.commit() + + resp = scope_restricted_authenticated_client.get("/reports/project", follow_redirects=True) + assert resp.status_code == 200 + data = resp.get_data(as_text=True) + # If we are on the report page (not redirected to login), project list should be scoped + on_login_page = "sign in" in data.lower() or "log in" in data.lower() + if not on_login_page: + # Allowed project (from test_client) should appear + assert project.name in data + # Disallowed project should not appear in the project list (scoped out) + assert "Other Project" not in data diff --git a/tests/test_routes/test_timer_scope.py b/tests/test_routes/test_timer_scope.py new file mode 100644 index 00000000..07eebb0f --- /dev/null +++ b/tests/test_routes/test_timer_scope.py @@ -0,0 +1,72 @@ +""" +Web timer scope tests: scope-restricted user can only start timer for allowed project. +""" + +import pytest + +from app import db + +pytestmark = [pytest.mark.routes, pytest.mark.integration] + + +def test_timer_start_denied_for_disallowed_project( + app, scope_restricted_authenticated_client, scope_restricted_user, project, test_client +): + """POST /timer/start with a project the user cannot access redirects with error or to login.""" + with app.app_context(): + from app.models import Client, Project + + other_client = Client(name="Other Corp", email="other@example.com") + other_client.status = "active" + db.session.add(other_client) + db.session.commit() + other_project = Project(name="Other Project", client_id=other_client.id, status="active") + db.session.add(other_project) + db.session.commit() + other_project_id = int(other_project.id) + + resp = scope_restricted_authenticated_client.post( + "/timer/start", + data={"project_id": other_project_id}, + follow_redirects=True, + ) + assert resp.status_code == 200 + data = resp.get_data(as_text=True) + # Either scope denial message, or login page (if session not established - still no timer on disallowed project) + assert ( + "do not have access" in data.lower() + or "access to this project" in data.lower() + or "log in" in data.lower() + or "sign in" in data.lower() + ) + # No active timer for that user on the disallowed project + with app.app_context(): + from app.models import TimeEntry + + active = TimeEntry.query.filter_by( + user_id=scope_restricted_user.id, project_id=other_project_id, end_time=None + ).first() + assert active is None + + +def test_timer_start_allowed_for_assigned_project( + app, scope_restricted_authenticated_client, scope_restricted_user, project +): + """POST /timer/start with an allowed project creates active timer when user is logged in.""" + project_id = int(project.id) + resp = scope_restricted_authenticated_client.post( + "/timer/start", + data={"project_id": project_id, "notes": "Scope test"}, + follow_redirects=True, + ) + assert resp.status_code == 200 + data = resp.get_data(as_text=True) + with app.app_context(): + from app.models import TimeEntry + + active = TimeEntry.query.filter_by( + user_id=scope_restricted_user.id, project_id=project_id, end_time=None + ).first() + # If we are logged in (not on login page), timer should have been started + if "sign in" not in data.lower() and "log in" not in data.lower(): + assert active is not None, "Expected active timer when scope-restricted user starts timer on allowed project" diff --git a/tests/test_services/test_recurring_invoice_service.py b/tests/test_services/test_recurring_invoice_service.py new file mode 100644 index 00000000..05a2a409 --- /dev/null +++ b/tests/test_services/test_recurring_invoice_service.py @@ -0,0 +1,25 @@ +""" +Tests for RecurringInvoiceService. +""" + +import pytest +from unittest.mock import MagicMock + +pytestmark = [pytest.mark.unit] + +from app.services.recurring_invoice_service import RecurringInvoiceService + + +class TestRecurringInvoiceService: + """Tests for RecurringInvoiceService.""" + + def test_generate_invoice_returns_none_when_should_not_generate(self): + """When should_generate_today() is False, generate_invoice returns None.""" + service = RecurringInvoiceService() + recurring = MagicMock() + recurring.should_generate_today.return_value = False + + result = service.generate_invoice(recurring) + + assert result is None + recurring.should_generate_today.assert_called_once() diff --git a/tests/test_utils.py b/tests/test_utils.py index e6ef6812..b7bb1433 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -631,17 +631,16 @@ def test_needs_compile_po_newer(): po_path = os.path.join(tmpdir, "messages.po") mo_path = os.path.join(tmpdir, "messages.mo") - # Create mo file first with open(mo_path, "wb") as f: f.write(b"old") - - # Wait a bit and create po file - import time - - time.sleep(0.01) with open(po_path, "w") as f: f.write("# new") + # Set deterministic mtimes: po newer than mo (no time.sleep) + base = 1000000000 + os.utime(mo_path, (base, base)) + os.utime(po_path, (base + 1, base + 1)) + assert _needs_compile(po_path, mo_path) is True @@ -653,17 +652,16 @@ def test_needs_compile_mo_current(): po_path = os.path.join(tmpdir, "messages.po") mo_path = os.path.join(tmpdir, "messages.mo") - # Create po file first with open(po_path, "w") as f: f.write("# test") - - # Wait and create mo file - import time - - time.sleep(0.01) with open(mo_path, "wb") as f: f.write(b"compiled") + # Set deterministic mtimes: mo newer than po (no time.sleep) + base = 1000000000 + os.utime(po_path, (base, base)) + os.utime(mo_path, (base + 1, base + 1)) + assert _needs_compile(po_path, mo_path) is False diff --git a/tests/test_utils/test_scope_filter.py b/tests/test_utils/test_scope_filter.py new file mode 100644 index 00000000..2ed5b33e --- /dev/null +++ b/tests/test_utils/test_scope_filter.py @@ -0,0 +1,237 @@ +""" +Tests for scope_filter utilities (issue access helper). +""" + +from datetime import datetime + +import pytest + +pytestmark = [pytest.mark.unit, pytest.mark.utils] + +from app.utils.scope_filter import ( + get_accessible_project_and_client_ids_for_user, + get_allowed_client_ids, + get_allowed_project_ids, + apply_client_scope_to_model, + apply_project_scope_to_model, + user_can_access_client, + user_can_access_project, +) +from app.models import User, Project, Task, Client, Role, UserClient +from app import db +from app.repositories import TimeEntryRepository + + +# --------------------------------------------------------------------------- +# get_accessible_project_and_client_ids_for_user (existing) +# --------------------------------------------------------------------------- + + +@pytest.mark.integration +def test_get_accessible_project_and_client_ids_for_user_empty(db_session): + """User with no time entries and no task assignments gets empty sets.""" + user = User(username="noprojects", role="user") + db_session.add(user) + db_session.commit() + + project_ids, client_ids = get_accessible_project_and_client_ids_for_user(user.id) + assert project_ids == set() + assert client_ids == set() + + +@pytest.mark.integration +def test_get_accessible_project_and_client_ids_for_user_via_time_entries(db_session): + """User with time entries gets those projects and their clients.""" + user = User(username="timer_user", role="user") + db_session.add(user) + client = Client(name="Acme") + db_session.add(client) + db_session.commit() + project = Project(name="P1", client_id=client.id, status="active") + db_session.add(project) + db_session.commit() + + repo = TimeEntryRepository() + repo.create_manual_entry( + user_id=user.id, + project_id=project.id, + start_time=datetime.now(), + end_time=datetime.now(), + ) + db_session.commit() + + project_ids, client_ids = get_accessible_project_and_client_ids_for_user(user.id) + assert project_ids == {project.id} + assert client_ids == {client.id} + + +# --------------------------------------------------------------------------- +# get_allowed_client_ids / get_allowed_project_ids +# --------------------------------------------------------------------------- + + +def test_get_allowed_client_ids_unrestricted_user(app, user): + """Non-scope-restricted user gets None (full access) from get_allowed_client_ids when passed explicitly.""" + with app.app_context(): + # user fixture has no subcontractor role + result = get_allowed_client_ids(user=user) + assert result is None + + +def test_get_allowed_project_ids_unrestricted_user(app, user): + """Non-scope-restricted user gets None (full access) from get_allowed_project_ids when passed explicitly.""" + with app.app_context(): + result = get_allowed_project_ids(user=user) + assert result is None + + +def test_get_allowed_client_ids_scope_restricted(app, scope_restricted_user, test_client): + """Scope-restricted user gets list of allowed client IDs.""" + with app.app_context(): + result = get_allowed_client_ids(user=scope_restricted_user) + assert result is not None + assert test_client.id in result + + +def test_get_allowed_project_ids_scope_restricted(app, scope_restricted_user, project): + """Scope-restricted user gets list of allowed project IDs (projects under assigned clients).""" + with app.app_context(): + result = get_allowed_project_ids(user=scope_restricted_user) + assert result is not None + assert project.id in result + + +# --------------------------------------------------------------------------- +# user_can_access_client / user_can_access_project +# --------------------------------------------------------------------------- + + +def test_user_can_access_client_admin(admin_user, test_client): + """Admin can access any client.""" + assert user_can_access_client(admin_user, test_client.id) is True + + +def test_user_can_access_client_unrestricted(user, test_client): + """Non-scope-restricted user can access any client.""" + assert user_can_access_client(user, test_client.id) is True + + +def test_user_can_access_client_scope_restricted_allowed(scope_restricted_user, test_client): + """Scope-restricted user can access assigned client.""" + assert user_can_access_client(scope_restricted_user, test_client.id) is True + + +def test_user_can_access_client_scope_restricted_denied(app, scope_restricted_user, test_client): + """Scope-restricted user cannot access non-assigned client.""" + with app.app_context(): + other = Client(name="Other Corp", email="other@example.com") + db.session.add(other) + db.session.commit() + assert user_can_access_client(scope_restricted_user, other.id) is False + + +def test_user_can_access_project_scope_restricted_allowed(scope_restricted_user, project): + """Scope-restricted user can access project under assigned client.""" + assert user_can_access_project(scope_restricted_user, project.id) is True + + +def test_user_can_access_project_scope_restricted_denied(app, scope_restricted_user, project, test_client): + """Scope-restricted user cannot access project under another client.""" + with app.app_context(): + other_client = Client(name="Other Corp", email="other@example.com") + db.session.add(other_client) + db.session.commit() + other_project = Project(name="Other Project", client_id=other_client.id, status="active") + db.session.add(other_project) + db.session.commit() + assert user_can_access_project(scope_restricted_user, other_project.id) is False + + +# --------------------------------------------------------------------------- +# apply_client_scope_to_model / apply_project_scope_to_model +# --------------------------------------------------------------------------- + + +def test_apply_client_scope_to_model_unrestricted(app, user): + """Unrestricted user: no filter applied (None).""" + with app.app_context(): + from app.models import Client as ClientModel + + scope = apply_client_scope_to_model(ClientModel, user=user) + assert scope is None + + +def test_apply_client_scope_to_model_scope_restricted(app, scope_restricted_user, test_client): + """Scope-restricted user: filter restricts to assigned clients only.""" + with app.app_context(): + from app.models import Client as ClientModel + + scope = apply_client_scope_to_model(ClientModel, user=scope_restricted_user) + assert scope is not None + q = ClientModel.query.filter(scope) + assert q.count() >= 1 + ids = [c.id for c in q.all()] + assert test_client.id in ids + + +def test_apply_project_scope_to_model_unrestricted(app, user): + """Unrestricted user: no filter applied (None).""" + with app.app_context(): + from app.models import Project as ProjectModel + + scope = apply_project_scope_to_model(ProjectModel, user=user) + assert scope is None + + +def test_apply_project_scope_to_model_scope_restricted(app, scope_restricted_user, project): + """Scope-restricted user: filter restricts to projects under assigned clients.""" + with app.app_context(): + from app.models import Project as ProjectModel + + scope = apply_project_scope_to_model(ProjectModel, user=scope_restricted_user) + assert scope is not None + q = ProjectModel.query.filter(scope) + assert q.count() >= 1 + ids = [p.id for p in q.all()] + assert project.id in ids + + +# --------------------------------------------------------------------------- +# Integration: API list filtered by scope-restricted user +# --------------------------------------------------------------------------- + + +@pytest.mark.integration +@pytest.mark.api +def test_api_projects_list_filtered_by_scope(app, scope_restricted_user, project, test_client): + """Scope-restricted user calling GET /api/v1/projects sees only projects for assigned client.""" + with app.app_context(): + other_client = Client(name="Other Corp", email="other@example.com") + db.session.add(other_client) + db.session.commit() + other_project = Project(name="Other Project", client_id=other_client.id, status="active") + db.session.add(other_project) + db.session.commit() + allowed_project_id = int(project.id) + other_project_id = int(other_project.id) + + from app.models import ApiToken + + token, plain = ApiToken.create_token( + user_id=scope_restricted_user.id, + name="Sub token", + scopes="read:projects", + expires_days=30, + ) + db.session.add(token) + db.session.commit() + + client = app.test_client() + client.environ_base["HTTP_AUTHORIZATION"] = f"Bearer {plain}" + resp = client.get("/api/v1/projects") + assert resp.status_code == 200 + data = resp.get_json() + assert "projects" in data + project_ids = [p["id"] for p in data["projects"]] + assert allowed_project_id in project_ids + assert other_project_id not in project_ids From 5eea34a97dd2f4ab7dafc26bbb850e4653d6217a Mon Sep 17 00:00:00 2001 From: Dries Peeters Date: Sun, 15 Mar 2026 09:39:28 +0100 Subject: [PATCH 4/4] Version Bump 4.23.0 --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index d1f29d5f..77ef910f 100644 --- a/setup.py +++ b/setup.py @@ -7,7 +7,7 @@ setup( name='timetracker', - version='4.22.2', + version='4.23.0', packages=find_packages(), include_package_data=True, install_requires=[