mirror of
https://github.com/DRYTRIX/TimeTracker.git
synced 2026-05-17 01:49:35 -05:00
fix(timer): respect Settings.single_active_timer at runtime
Timer starts always blocked a second running entry and never read the\nadmin-controlled Settings flag.\n\n- Add TimeTrackingService.can_start_timer() using Settings.get_settings()\n and wire it into start_timer, web timer routes, kiosk start, and\n legacy POST /api/timer/resume.\n- POST /api/v1/timer/start returns 409 with error_code\n timer_already_running when single-active mode is on and a timer\n is already running.\n- Deduplicate start_timer template handling in the service.\n\nTests: tests/test_single_active_timer_setting.py.\nDocs: REST_API (responses), GETTING_STARTED, REQUIREMENTS, Docker env\nnotes, TESTING_STRATEGY, env.example comment; CHANGELOG entry.
This commit is contained in:
Dries Peeters
@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
- **LDAP authentication** — Optional directory login via `AUTH_METHOD=ldap` or combined `AUTH_METHOD=all` (with local + OIDC). New `LDAP_*` settings in `app/config.py`, `LDAPService` (`app/services/ldap_service.py`), login and password-reset behaviour keyed off `users.auth_provider` (`local` | `oidc` | `ldap`), admin **System Settings** LDAP panel and `POST /admin/ldap/test`, production env validation for required LDAP variables, Alembic `153_add_user_auth_provider`, and tests in `tests/test_ldap_auth.py`. Dependency: `ldap3`. Documentation: [docs/admin/configuration/LDAP_SETUP.md](docs/admin/configuration/LDAP_SETUP.md); OIDC and getting-started guides updated for `ldap` / `all`.
|
||||
|
||||
### Fixed
|
||||
- **Admin “Allow only one active timer per user” ignored at runtime** — Timer start and related flows always blocked a second running entry and never read `Settings.single_active_timer` from the database. Enforcement now uses `Settings.get_settings()` via `TimeTrackingService.can_start_timer` (web timer routes, REST v1, kiosk start, legacy session `POST /api/timer/resume`). `POST /api/v1/timer/start` returns **409** with `error_code: timer_already_running` when the setting is on and a timer is already running. `SINGLE_ACTIVE_TIMER` still seeds new installs only. Tests: `tests/test_single_active_timer_setting.py`.
|
||||
- **API integration test for project tasks** — `tests/test_api_comprehensive.py` now matches `GET /api/projects/<id>/tasks`, which returns **all** tasks (including done and cancelled) for the time-entry UI.
|
||||
- **Quote create returned HTTP 500 after save (#583)** — The quote was saved, but the redirect to the quote detail page crashed when **Valid until** was set: the template compared `valid_until` to `now()`, and `now` was never defined in the Jinja context. The expired badge now uses `Quote.is_expired` (same rule, app timezone). Regression coverage in `tests/test_routes/test_quotes_web.py` posts `valid_until` so the view path is exercised.
|
||||
- **Desktop app navigation guard** — `will-navigate` no longer mis-classifies `file:` loads (opaque `"null"` origin) as external navigation. Allowed in-app protocols include `file:`, `about:`, and `devtools:`; `http:` / `https:` are still blocked from the embedded window.
|
||||
|
||||
+2
-1
@@ -447,7 +447,8 @@ def api_stop_timer_at():
|
||||
@deprecated_session_api("/api/v1/timer/start")
|
||||
def api_resume_timer():
|
||||
"""Resume timer for last used project/task or provided project/task."""
|
||||
if current_user.active_timer:
|
||||
can_start, _ = TimeTrackingService().can_start_timer(current_user.id)
|
||||
if not can_start:
|
||||
return jsonify({"error": "Timer already running"}), 400
|
||||
|
||||
data = request.get_json() or {}
|
||||
|
||||
@@ -374,6 +374,12 @@ def start_timer():
|
||||
template_id=data.get("template_id"),
|
||||
)
|
||||
if not result.get("success"):
|
||||
if result.get("error") == "timer_already_running":
|
||||
return error_response(
|
||||
result.get("message", "Could not start timer"),
|
||||
error_code="timer_already_running",
|
||||
status_code=409,
|
||||
)
|
||||
return error_response(
|
||||
result.get("message", "Could not start timer"),
|
||||
status_code=400,
|
||||
|
||||
+3
-3
@@ -10,6 +10,7 @@ from sqlalchemy import func, or_
|
||||
|
||||
from app import db, log_event
|
||||
from app.models import Project, Settings, StockItem, StockMovement, Task, TimeEntry, User, Warehouse, WarehouseStock
|
||||
from app.services.time_tracking_service import TimeTrackingService
|
||||
from app.utils.db import safe_commit
|
||||
from app.utils.module_helpers import module_enabled
|
||||
from app.utils.permissions import admin_or_permission_required
|
||||
@@ -469,9 +470,8 @@ def kiosk_start_timer():
|
||||
if not user_can_access_project(current_user, project_id):
|
||||
return jsonify({"error": "You do not have access to this project"}), 403
|
||||
|
||||
# Check if user already has an active timer
|
||||
active_timer = current_user.active_timer
|
||||
if active_timer:
|
||||
can_start, _ = TimeTrackingService().can_start_timer(current_user.id)
|
||||
if not can_start:
|
||||
return jsonify({"error": "You already have an active timer"}), 400
|
||||
|
||||
# Validate task if provided
|
||||
|
||||
+9
-12
@@ -12,6 +12,7 @@ from app.constants import TimeEntrySource
|
||||
from app.models import Activity, Client, Project, Settings, Task, TimeEntry, User
|
||||
from app.services.client_service import ClientService
|
||||
from app.services.project_service import ProjectService
|
||||
from app.services.time_tracking_service import TimeTrackingService
|
||||
from app.utils.db import safe_commit
|
||||
from app.utils.error_handling import safe_log
|
||||
from app.utils.posthog_funnels import track_onboarding_first_time_entry, track_onboarding_first_timer
|
||||
@@ -171,9 +172,8 @@ def start_timer():
|
||||
current_app.logger.warning("Start timer denied: user has no access to client_id=%s", client_id)
|
||||
return redirect(url_for("main.dashboard"))
|
||||
|
||||
# Check if user already has an active timer
|
||||
active_timer = current_user.active_timer
|
||||
if active_timer:
|
||||
can_start, _ = TimeTrackingService().can_start_timer(current_user.id)
|
||||
if not can_start:
|
||||
flash(_("You already have an active timer. Stop it before starting a new one."), "error")
|
||||
current_app.logger.info("Start timer blocked: user already has an active timer")
|
||||
return redirect(url_for("main.dashboard"))
|
||||
@@ -336,9 +336,8 @@ def start_timer_from_template(template_id):
|
||||
# Load template
|
||||
template = TimeEntryTemplate.query.filter_by(id=template_id, user_id=current_user.id).first_or_404()
|
||||
|
||||
# Check if user already has an active timer
|
||||
active_timer = current_user.active_timer
|
||||
if active_timer:
|
||||
can_start, _ = TimeTrackingService().can_start_timer(current_user.id)
|
||||
if not can_start:
|
||||
flash(_("You already have an active timer. Stop it before starting a new one."), "error")
|
||||
return redirect(url_for("main.dashboard"))
|
||||
|
||||
@@ -451,9 +450,8 @@ def start_timer_for_project(project_id):
|
||||
current_app.logger.warning("Start timer (GET) denied: user has no access to project_id=%s", project_id)
|
||||
return redirect(url_for("main.dashboard"))
|
||||
|
||||
# Check if user already has an active timer
|
||||
active_timer = current_user.active_timer
|
||||
if active_timer:
|
||||
can_start, _ = TimeTrackingService().can_start_timer(current_user.id)
|
||||
if not can_start:
|
||||
flash(_("You already have an active timer. Stop it before starting a new one."), "error")
|
||||
current_app.logger.info("Start timer (GET) blocked: user already has an active timer")
|
||||
return redirect(url_for("main.dashboard"))
|
||||
@@ -2021,9 +2019,8 @@ def resume_timer_by_id(timer_id):
|
||||
flash(_("You can only resume your own timers"), "error")
|
||||
return redirect(url_for("main.dashboard"))
|
||||
|
||||
# Check if user already has an active timer
|
||||
active_timer = current_user.active_timer
|
||||
if active_timer:
|
||||
can_start, _ = TimeTrackingService().can_start_timer(current_user.id)
|
||||
if not can_start:
|
||||
flash("You already have an active timer. Stop it before resuming another one.", "error")
|
||||
current_app.logger.info("Resume timer blocked: user already has an active timer")
|
||||
return redirect(url_for("main.dashboard"))
|
||||
|
||||
@@ -34,6 +34,19 @@ class TimeTrackingService:
|
||||
end_time=end_time,
|
||||
)
|
||||
|
||||
def can_start_timer(self, user_id: int) -> tuple[bool, Optional[str]]:
|
||||
"""Return (True, None) if the user may start a new timer, else (False, message).
|
||||
|
||||
Reads ``Settings.get_settings()`` at call time (DB), not ``Config.SINGLE_ACTIVE_TIMER``
|
||||
alone—env seeds new installs; admin UI updates the row users expect at runtime.
|
||||
"""
|
||||
settings = Settings.get_settings()
|
||||
if not settings.single_active_timer:
|
||||
return True, None
|
||||
if self.time_entry_repo.get_active_timer(user_id):
|
||||
return False, "You already have an active timer. Stop it before starting a new one."
|
||||
return True, None
|
||||
|
||||
def start_timer(
|
||||
self,
|
||||
user_id: int,
|
||||
@@ -48,29 +61,6 @@ class TimeTrackingService:
|
||||
Returns:
|
||||
dict with 'success', 'message', and 'timer' keys
|
||||
"""
|
||||
# Load template if provided
|
||||
if template_id:
|
||||
from app.models import TimeEntryTemplate
|
||||
|
||||
template = TimeEntryTemplate.query.filter_by(id=template_id, user_id=user_id).first()
|
||||
if template:
|
||||
# Override with template values if not explicitly set
|
||||
if not project_id and template.project_id:
|
||||
project_id = template.project_id
|
||||
if not task_id and template.task_id:
|
||||
task_id = template.task_id
|
||||
if not notes and template.default_notes:
|
||||
notes = template.default_notes
|
||||
# Mark template as used
|
||||
template.record_usage()
|
||||
db.session.commit()
|
||||
"""
|
||||
Start a new timer for a user.
|
||||
|
||||
Returns:
|
||||
dict with 'success', 'message', and 'timer' keys
|
||||
"""
|
||||
# Check if user already has an active timer
|
||||
if self._is_locked_period(user_id, local_now()):
|
||||
return {
|
||||
"success": False,
|
||||
@@ -78,14 +68,28 @@ class TimeTrackingService:
|
||||
"error": "timesheet_period_locked",
|
||||
}
|
||||
|
||||
active_timer = self.time_entry_repo.get_active_timer(user_id)
|
||||
if active_timer:
|
||||
ok, conflict_msg = self.can_start_timer(user_id)
|
||||
if not ok:
|
||||
return {
|
||||
"success": False,
|
||||
"message": "You already have an active timer. Stop it before starting a new one.",
|
||||
"message": conflict_msg or "You already have an active timer. Stop it before starting a new one.",
|
||||
"error": "timer_already_running",
|
||||
}
|
||||
|
||||
# Resolve template defaults before project validation
|
||||
if template_id:
|
||||
from app.models import TimeEntryTemplate
|
||||
|
||||
template = TimeEntryTemplate.query.filter_by(id=template_id, user_id=user_id).first()
|
||||
if template:
|
||||
if not project_id and template.project_id:
|
||||
project_id = template.project_id
|
||||
if not task_id and template.task_id:
|
||||
task_id = template.task_id
|
||||
if not notes and template.default_notes:
|
||||
notes = template.default_notes
|
||||
template.record_usage()
|
||||
|
||||
# Validate project
|
||||
project = self.project_repo.get_by_id(project_id)
|
||||
if not project:
|
||||
@@ -106,20 +110,6 @@ class TimeTrackingService:
|
||||
"error": "project_inactive",
|
||||
}
|
||||
|
||||
# Load template if provided
|
||||
if template_id:
|
||||
from app.models import TimeEntryTemplate
|
||||
|
||||
template = TimeEntryTemplate.query.filter_by(id=template_id, user_id=user_id).first()
|
||||
if template:
|
||||
if not project_id and template.project_id:
|
||||
project_id = template.project_id
|
||||
if not task_id and template.task_id:
|
||||
task_id = template.task_id
|
||||
if not notes and template.default_notes:
|
||||
notes = template.default_notes
|
||||
template.record_usage()
|
||||
|
||||
# Validate task if provided
|
||||
if task_id:
|
||||
task = Task.query.filter_by(id=task_id, project_id=project_id).first()
|
||||
|
||||
@@ -153,7 +153,7 @@ The Admin Settings page has multiple sections. Configure what you need:
|
||||
#### Timer Settings
|
||||
- **Rounding (Minutes)**: Round to nearest 1/5/15 minutes
|
||||
- **Idle Timeout (Minutes)**: Auto-pause after idle (default: 30)
|
||||
- **Single Active Timer**: Allow only one running timer per user
|
||||
- **Single Active Timer**: When enabled (default), a user cannot start another timer until the current running entry is stopped. When disabled, multiple concurrent timers are allowed. The value is stored in the database (**System Settings**); the `SINGLE_ACTIVE_TIMER` environment variable only seeds that row on first install—after that, changes in the admin UI apply immediately to web, REST v1, and kiosk timer starts.
|
||||
|
||||
#### User Management
|
||||
- **Allow Self-Registration**: ☑ Enable this to let users create accounts by entering any username and password on the login page. When enabled, anyone can create an app user with whatever credentials they type—there is no link to database credentials. **Security note**: Avoid using your database username (e.g. `timetracker`) as an app username, and do not share database passwords. With self-register enabled, someone could create an app account with credentials that match your DB user, which can be confusing or a security risk.
|
||||
|
||||
@@ -101,7 +101,7 @@ A Python backend (Flask recommended) runs inside Docker on a Raspberry Pi. The f
|
||||
2. **Server-Side Persistence:** Timer continues running on the server even if the browser closes, device sleeps, or network drops.
|
||||
3. **Stop Timer:** User clicks **Stop** from any browser session or device; server finalizes entry (start->stop).
|
||||
4. **Resilience:** If the RPI restarts, active timers are restored using last known start time and a flag indicating “active”.
|
||||
5. **Single Active Timer per User:** Enforced; starting a new timer stops the previous one (configurable).
|
||||
5. **Single Active Timer per User:** By default only one running timer per user is allowed; attempting to start another while one is running is rejected until the first is stopped. **System Settings** can disable this to allow multiple concurrent timers. The `SINGLE_ACTIVE_TIMER` environment variable seeds the initial stored value for new deployments; runtime enforcement follows the database setting.
|
||||
6. **Idle/AFK (optional v1.1):** After N minutes of inactivity, prompt on next visit to confirm whether to subtract idle time.
|
||||
|
||||
### 5.5 Time Entry Annotations & Metadata
|
||||
|
||||
@@ -80,7 +80,7 @@ All environment variables can be provided via `.env` and are consumed by the `ap
|
||||
### Application behavior
|
||||
- CURRENCY: ISO currency code. Default: `EUR`.
|
||||
- ROUNDING_MINUTES: Rounding step for entries. Default: `1`.
|
||||
- SINGLE_ACTIVE_TIMER: Allow only one active timer per user. Default: `true`.
|
||||
- SINGLE_ACTIVE_TIMER: Seeds **allow only one active timer per user** for the initial settings row. Default: `true`. After install, **System Settings → Settings** updates the database value used at runtime (web, API v1, kiosk).
|
||||
- IDLE_TIMEOUT_MINUTES: Auto-pause after idle. Default: `30`.
|
||||
- ALLOW_SELF_REGISTER: Allow new users to self-register by entering any username and password on the login page. Default: `true`. **Security note**: When enabled, anyone can create an app user with whatever credentials they type. The app does not use or import database credentials—users are created with exactly what is entered. Avoid using your database username (e.g. `timetracker`) as an app username; if someone creates an app user with matching DB credentials, it can be confusing or a security risk.
|
||||
- ADMIN_USERNAMES: Comma-separated admin usernames. Default: `admin`. **Important**: Only the first username in the list is automatically created during database initialization. Additional admin usernames must either:
|
||||
|
||||
@@ -620,6 +620,13 @@ POST /api/v1/timer/start
|
||||
}
|
||||
```
|
||||
|
||||
**Responses:**
|
||||
- **`201 Created`** — Timer started; JSON includes `message` and `timer` (time entry fields).
|
||||
- **`409 Conflict`** — **Allow only one active timer per user** is enabled in **System Settings** (`single_active_timer`) and the user already has a running timer. Response uses the standard error shape with `error_code` set to `timer_already_running`.
|
||||
- **`400 Bad Request`** — Validation or other errors (e.g. invalid project, inactive project).
|
||||
|
||||
Enforcement uses the persisted **Settings** row, not the `SINGLE_ACTIVE_TIMER` env var alone (the env var seeds the setting on first install).
|
||||
|
||||
#### Stop Timer
|
||||
```
|
||||
POST /api/v1/timer/stop
|
||||
|
||||
@@ -22,7 +22,7 @@ Tests must cover:
|
||||
| Area | What to test |
|
||||
|------|--------------|
|
||||
| **Auth** | Web login (GET/POST, success, wrong password, redirect); API token extraction, validation, scopes, IP whitelist. |
|
||||
| **Timer** | Start/stop via web (POST /timer/start, /timer/stop) and API; single active timer; scope (user can only start timer for allowed project). |
|
||||
| **Timer** | Start/stop via web (POST /timer/start, /timer/stop) and API; `Settings.single_active_timer` respected on start (web, `POST /api/v1/timer/start`, kiosk); see `tests/test_single_active_timer_setting.py`. Scope: user can only start a timer for an allowed project. |
|
||||
| **Time entries** | CRUD and edit (API PATCH, web if applicable); linking to project/client/task; duration and billable. |
|
||||
| **Project/client linking** | Project belongs to client; scope filter restricts visible projects/clients for subcontractors. |
|
||||
| **Invoicing** | Create invoice from time entries; recurring invoices; list/detail; payments. |
|
||||
|
||||
@@ -28,6 +28,7 @@ PERMANENT_SESSION_LIFETIME=86400
|
||||
TZ=Europe/Rome
|
||||
CURRENCY=EUR
|
||||
ROUNDING_MINUTES=1
|
||||
# Seeds Settings.single_active_timer on first settings row; admin System Settings overrides thereafter.
|
||||
SINGLE_ACTIVE_TIMER=true
|
||||
IDLE_TIMEOUT_MINUTES=30
|
||||
|
||||
|
||||
@@ -0,0 +1,144 @@
|
||||
"""Tests for Settings.single_active_timer enforcement (DB) vs env defaults."""
|
||||
|
||||
import json
|
||||
from datetime import datetime
|
||||
from decimal import Decimal
|
||||
|
||||
import pytest
|
||||
|
||||
from app import db
|
||||
from app.models import Project, Settings, TimeEntry
|
||||
|
||||
pytestmark = [pytest.mark.integration]
|
||||
|
||||
|
||||
def _api_headers(plain_token: str) -> dict:
|
||||
return {"Authorization": f"Bearer {plain_token}", "Content-Type": "application/json"}
|
||||
|
||||
|
||||
def _second_project(client_id: int) -> Project:
|
||||
p = Project(
|
||||
name="Second Timer Project",
|
||||
client_id=client_id,
|
||||
description="second",
|
||||
billable=True,
|
||||
hourly_rate=Decimal("80.00"),
|
||||
status="active",
|
||||
)
|
||||
db.session.add(p)
|
||||
db.session.flush()
|
||||
return p
|
||||
|
||||
|
||||
def test_single_timer_enforced_when_setting_on(app, client, user, project, api_token):
|
||||
_, plain_token = api_token
|
||||
p2 = _second_project(project.client_id)
|
||||
db.session.commit()
|
||||
|
||||
with app.app_context():
|
||||
settings = Settings.get_settings()
|
||||
settings.single_active_timer = True
|
||||
db.session.commit()
|
||||
|
||||
running = TimeEntry(
|
||||
user_id=user.id,
|
||||
project_id=project.id,
|
||||
start_time=datetime.utcnow(),
|
||||
end_time=None,
|
||||
source="manual",
|
||||
billable=True,
|
||||
)
|
||||
db.session.add(running)
|
||||
db.session.commit()
|
||||
|
||||
resp = client.post(
|
||||
"/api/v1/timer/start",
|
||||
json={"project_id": p2.id},
|
||||
headers=_api_headers(plain_token),
|
||||
)
|
||||
assert resp.status_code == 409
|
||||
data = json.loads(resp.data)
|
||||
assert data.get("success") is False
|
||||
assert data.get("error_code") == "timer_already_running"
|
||||
|
||||
|
||||
def test_multiple_timers_allowed_when_setting_off(app, client, user, project, api_token):
|
||||
_, plain_token = api_token
|
||||
p2 = _second_project(project.client_id)
|
||||
db.session.commit()
|
||||
|
||||
with app.app_context():
|
||||
settings = Settings.get_settings()
|
||||
settings.single_active_timer = False
|
||||
db.session.commit()
|
||||
|
||||
r1 = client.post("/api/v1/timer/start", json={"project_id": project.id}, headers=_api_headers(plain_token))
|
||||
assert r1.status_code == 201
|
||||
|
||||
r2 = client.post("/api/v1/timer/start", json={"project_id": p2.id}, headers=_api_headers(plain_token))
|
||||
assert r2.status_code == 201
|
||||
|
||||
with app.app_context():
|
||||
active = TimeEntry.query.filter_by(user_id=user.id, end_time=None).all()
|
||||
assert len(active) == 2
|
||||
|
||||
|
||||
def test_setting_read_from_db_not_env(app, client, user, project, api_token):
|
||||
"""DB single_active_timer=False must allow a second timer even if env default is restrictive."""
|
||||
_, plain_token = api_token
|
||||
p2 = _second_project(project.client_id)
|
||||
db.session.commit()
|
||||
|
||||
with app.app_context():
|
||||
settings = Settings.get_settings()
|
||||
settings.single_active_timer = False
|
||||
db.session.commit()
|
||||
|
||||
running = TimeEntry(
|
||||
user_id=user.id,
|
||||
project_id=project.id,
|
||||
start_time=datetime.utcnow(),
|
||||
end_time=None,
|
||||
source="manual",
|
||||
billable=True,
|
||||
)
|
||||
db.session.add(running)
|
||||
db.session.commit()
|
||||
|
||||
resp = client.post(
|
||||
"/api/v1/timer/start",
|
||||
json={"project_id": p2.id},
|
||||
headers=_api_headers(plain_token),
|
||||
)
|
||||
assert resp.status_code == 201
|
||||
with app.app_context():
|
||||
assert TimeEntry.query.filter_by(user_id=user.id, end_time=None).count() == 2
|
||||
|
||||
|
||||
def test_both_web_and_api_routes_respect_setting(app, authenticated_client, user, project, api_token):
|
||||
_, plain_token = api_token
|
||||
p2 = _second_project(project.client_id)
|
||||
db.session.commit()
|
||||
|
||||
with app.app_context():
|
||||
settings = Settings.get_settings()
|
||||
settings.single_active_timer = False
|
||||
db.session.commit()
|
||||
|
||||
web_resp = authenticated_client.post(
|
||||
"/timer/start",
|
||||
data={"project_id": str(project.id)},
|
||||
follow_redirects=True,
|
||||
)
|
||||
assert web_resp.status_code == 200
|
||||
|
||||
api_resp = authenticated_client.post(
|
||||
"/api/v1/timer/start",
|
||||
json={"project_id": p2.id},
|
||||
headers=_api_headers(plain_token),
|
||||
)
|
||||
assert api_resp.status_code == 201
|
||||
|
||||
with app.app_context():
|
||||
active = TimeEntry.query.filter_by(user_id=user.id, end_time=None).all()
|
||||
assert len(active) == 2
|
||||
Reference in New Issue
Block a user