`test_oidc_callback_does_not_store_id_token_in_cookie_session` has been
failing on every CI run with:
AssertionError: assert 'oidc_id_token_key' in
<SecureCookieSession {'_flashes': [('error',
'User account does not exist and self-registration is disabled.')]}>
The OIDC callback flow needs to (a) find or create a User matching the
incoming token's identity, then (b) store the id_token server-side and
put only a short key in the session. The test exercises path (b), but
in the test environment `ALLOW_SELF_REGISTER` is false (the production-
safe default in `app/config.py:45`), so step (a) flashes the
"self-registration disabled" error and short-circuits with a redirect
to /login before the token-storage code under test even runs.
Fix: pre-create a `User` row matching the fake OIDC subject's username
+ email so step (a) finds the user and step (b) executes. The test is
about session bloat, not the user-creation path; making it independent
of the `ALLOW_SELF_REGISTER` setting keeps it focused.
Test plan
- pytest tests/test_oidc_session_cookie_bloat.py::test_oidc_callback_does_not_store_id_token_in_cookie_session
20 test files share an anti-pattern: each declares its own local `app`
fixture with a hardcoded SQLite filename in the test runner's CWD,
e.g.
"SQLALCHEMY_DATABASE_URI": "sqlite:///test_api_kanban.sqlite",
Two problems:
1. If a prior test run crashed before teardown, the .sqlite file
persists. The next run starts on a non-empty database and trips
over FK / uniqueness constraints — `test_api_tax_currency_flow`
surfaces this as a 409 Conflict when its USD currency row already
exists.
2. `test_api_invoice_templates.sqlite` is used by two files
(`test_api_invoice_templates_v1.py` and
`test_api_invoice_templates_api_v1.py`). Under pytest-xdist they
race for the same file and corrupt each other's state.
Fix: each fixture now generates a unique per-call path using the same
pattern already established in `tests/conftest.py:204`:
unique_db_path = os.path.join(
tempfile.gettempdir(), f"test_<slug>_{uuid.uuid4().hex}.sqlite"
)
"SQLALCHEMY_DATABASE_URI": f"sqlite:///{unique_db_path}",
Adds `import os`, `import tempfile`, `import uuid` where missing.
Test lifecycle is unchanged — the existing fixture teardown
(`db.drop_all()` + engine dispose) still runs; the OS later reaps the
temp file. Black-formatted to the project's 120-char line limit.
Test plan
- pytest tests/test_api_kanban_v1.py
- pytest tests/test_api_tax_currency_v1.py (no leftover-USD 409)
- pytest -p xdist -n 2 tests/test_api_invoice_templates_v1.py tests/test_api_invoice_templates_api_v1.py (no collision)
The Expense model has a `notes` Text column and `Expense.to_dict()`
returns it, but both the PATCH route handler and the
`ExpenseService.update_expense` allowlist tuples omit `notes`. As a
result, callers can send `{"notes": "..."}` in a PUT/PATCH body and
get a 200 back with the field silently dropped — exactly what
`tests/test_api_expenses_v1.py::test_expenses_crud` asserts against
when it sends `notes="airport ride"` and reads back `None`.
Add `"notes"` to the two allowlist tuples (one in the route, one in
the service). No model or schema change required.
Test plan
- pytest tests/test_api_expenses_v1.py::test_expenses_crud
- pytest tests/test_routes/test_api_v1_expenses_complete.py
Four integration tests fell behind code changes in app/integrations/ and
app/models. They were asserting against the old data shapes / patching
the old mock targets and only surfaced now that v5.5.6's fixture refresh
re-enabled them.
### tests/test_integration/test_caldav_integration.py
test_sync_data_imports_events still expected the connector to write
TimeEntry rows and IntegrationExternalEventLink rows. The connector
in app/integrations/caldav_calendar.py:803 now writes CalendarEvent
rows and tracks imports via a [CalDAV: <uid>] marker embedded in the
description. Update the assertions to query CalendarEvent and look for
the marker. Add CalendarEvent to the model imports.
### tests/test_integration/test_activitywatch_integration.py
Four tests in this file patched `app.integrations.activitywatch.requests.get`.
The connector at app/integrations/activitywatch.py:78-79 was refactored
to route HTTP through `integration_session()` + `session_request()`,
so requests.get is never called and the mocks never fire. Tests that
relied on the mock then attempted real HTTP to localhost:5600 and
failed.
Repoint all four @patch decorators to
`app.integrations.activitywatch.session_request`. Update the one
positional-argument assertion in test_test_connection_success — the
URL moves from arg[0] to arg[2] because session_request's signature
is (session, method, url, **kw).
### tests/test_custom_field_definitions.py
test_delete_custom_field_preserves_other_fields called
`test_client.set_custom_field(...)` directly on the fixture instance
which may belong to a different SQLAlchemy session by the time the
test runs. The two JSON mutations weren't reliably persisted, so only
one of the two fields survived to the assertions. Re-query the client
via `Client.query.get(test_client.id)` before mutating — matches the
pattern already used in the surrounding tests in the same file.
Test plan
- pytest tests/test_integration/test_caldav_integration.py::TestCalDAVConnector::test_sync_data_imports_events
- pytest tests/test_integration/test_activitywatch_integration.py::TestActivityWatchConnector
- pytest tests/test_custom_field_definitions.py::test_delete_custom_field_preserves_other_fields
Five integration tests fail because three API v1 route groups call
`joinedload()` on relationships the underlying models never define,
producing AttributeError -> 500 on the first query.
The three bad references were all introduced in commit 579fc7af
("extract business logic to service layer", Nov 2025) and have been
quietly broken since. They surfaced only now because the v5.5.6
fixture refresh started exercising these endpoints more thoroughly.
FK enforcement (also added in v5.5.6) is a red herring here.
### Repairs
- `KanbanColumn` has no `project` relationship (only the `project_id`
FK column), and `KanbanColumn.to_dict()` never reads `self.project`.
The eager load is dead code in both the PUT/PATCH and DELETE
handlers — drop it.
- `ClientNote` has an `author` relationship, not `created_by_user`,
and `ClientNote.to_dict()` reads `self.author.username` /
`self.author.full_name`. The four call sites still need eager
loading; rename the attribute to the real one (`author`).
- `SavedFilter` has no `user` relationship (only the `user_id` FK
column), and `SavedFilter.to_dict()` never reads `self.user`. Like
KanbanColumn, the eager load is dead code in all four call sites —
drop it.
### Tests fixed
- tests/test_api_kanban_v1.py::test_kanban_columns
- tests/test_api_client_notes_v1.py::test_client_notes_crud
- tests/test_api_saved_filters_v1.py::test_saved_filters_crud
- tests/test_routes/test_api_v1_invoices_tasks_expenses_refactored.py::TestAPITasksRefactored::test_get_task_uses_eager_loading (transitively, via the same route module)
### Notes
The unused `from sqlalchemy.orm import joinedload` import in the
kanban/saved-filter handlers is left in place to keep the diff
surgical. It can be removed in a follow-up cleanup.
### Test plan
- [ ] pytest tests/test_api_kanban_v1.py::test_kanban_columns
- [ ] pytest tests/test_api_client_notes_v1.py::test_client_notes_crud
- [ ] pytest tests/test_api_saved_filters_v1.py::test_saved_filters_crud
- [ ] All three return 200 from CRUD operations; no AttributeError logged
- [ ] No regression on routes that already used joinedload correctly
Three tests in `test_timer_edit_own_time_entries.py` failed with
`sqlalchemy.exc.InvalidRequestError: Object '<Role at 0x...>' is not
attached to a Session` when the suite ran after v5.5.6's fixture
refresh.
Root cause: the `user` fixture commits the user and calls
`db.session.refresh(user)`, leaving the instance bound to the
fixture's scoped session. Each test then enters a fresh
`with app.app_context():` block before calling
`_ensure_edit_own_permission(user)`. Flask-SQLAlchemy's scoped session
in the new context is a *different* session than the one that owns
`user`, so when `user.add_role(role)` tries to associate a freshly
loaded `Role` with the cross-session `user`, the resulting commit
detaches the role and the next attribute access fails.
Fix: have `_ensure_edit_own_permission` take a `user_id` instead of the
ORM instance and re-query the user from the active session
(`User.query.get(user_id)`). All three objects (user, role, permission)
now live in the same identity map. Drop the trailing
`db.session.refresh(user)` — route handlers re-load the user via their
own request-scoped sessions and see the persisted association.
Test plan
- pytest tests/test_timer_edit_own_time_entries.py
Bundles four narrow fixes surfaced by the comprehensive CI run after
v5.5.6 enabled SQLite FK enforcement and refreshed test fixtures. Each
fix is independent; bundled here only to keep PR overhead low.
1. tests/test_routes/test_api_v1_expenses_complete.py
- Add missing `User` import. `test_expense_permissions` references
`User.query.filter(...)` but `User` was never imported, causing a
NameError at test runtime.
2. tests/test_client_single_simplification.py
- `Client(...status="active")` failed with TypeError because the
Client model's __init__ does not accept `status` as a keyword
(column default already sets it to "active"). Set the attribute
after construction.
3. tests/test_utils/test_api_auth_enhanced.py
- `User(username=..., is_active=True)` failed because User.__init__
does not accept `is_active`. Set the attribute after construction.
- 9 occurrences of `app.test_request_context(remote_addr="...")`
failed with `TypeError: EnvironBuilder.__init__() got an unexpected
keyword argument 'remote_addr'`. Werkzeug removed the keyword;
replace with `environ_overrides={"REMOTE_ADDR": "..."}` which is
the supported equivalent.
4. app/routes/api_v1_time_entries.py
- DELETE /api/v1/time-entries/<id> returned 415 Unsupported Media
Type when called without an `application/json` Content-Type, even
though the body is optional (only used to capture an audit reason).
Switch to `request.get_json(silent=True)` so the endpoint accepts
DELETE requests with no body. Same change applied to no other
methods; POST/PUT continue to require explicit JSON.
The route file also picked up a black/isort pass from the project
auto-formatter; behaviour is identical to before, only whitespace and
import grouping differ.
Test plan
- pytest tests/test_routes/test_api_v1_expenses_complete.py::TestAPIExpensesComplete::test_expense_permissions
- pytest tests/test_client_single_simplification.py::test_manual_entry_shows_select_when_multiple_clients
- pytest tests/test_utils/test_api_auth_enhanced.py::TestAuthenticateToken
- pytest tests/test_routes/test_api_v1_time_entries_complete.py::TestAPITimeEntriesComplete::test_delete_time_entry_uses_service_layer
Bump setup.py to 5.5.7 and document the invoice PDF designer layout fix,
designer-to-ReportLab template parity, and preview vs export alignment
for issue #622 in CHANGELOG.md.
Close the missing canvas-area wrapper in the invoice PDF designer so the properties panel sits in the third grid column beside the canvas instead of below it.
When saving template JSON from the designer, read items-table and expenses-table width, colors, and separator line settings from the Konva group children so exports match user edits. Scale column widths to the chosen table width and emit a style block for ReportLab.
In the ReportLab renderer, scale column widths to element.width, wrap tables in a two-column outer table to honor horizontal x offset relative to the left margin, and apply borderColor and borderWidth from template style.
Extend the JSON-to-HTML preview converter so table header text, row text, row background, and border width reflect the same style keys used on export (fixes preview vs PDF mismatch for issue #622).
The test file uses pytestmark = [pytest.mark.integration] on line 5 but
does not import pytest, causing pytest collection to fail with
NameError: name 'pytest' is not defined. Add the missing import.
- Honor AI_ENABLED across session AI, REST v1, LLM service, templates, and
context; add regression tests for the AI helper gate.
- Docker Compose: optional Ollama behind the ai profile; align env.example
and example compose with safe defaults.
- Add UNINSTALL.md with a dedicated AI teardown section; cross-link from
README, INSTALLATION, Getting Started, docs index, and Docker setup guide.
- Record 5.5.6 in CHANGELOG and sync version examples in BUILD_CONFIGURATION;
bump setup.py to 5.5.6.
The dashboard route now writes two cache entries per user
(dashboard:stats:<id> and dashboard💹<id>) instead of a single
dashboard:<id> blob, and today_hours moved into the nested
time_tracking payload. Update the caching tests to mirror this shape
and centralise the key tuple in a small helper.
The ProjectCost.updated_at column default captures datetime.utcnow at
class definition time, so patching app.models.project_cost.datetime
does not change the initial value written on insert. Assign
updated_at directly on the instance to get the deterministic
"before" timestamp the test relies on.
base.html now loads keyboard-shortcuts-advanced.js globally instead of
keyboard-shortcuts-enhanced.js, and the cheat-sheet CSS is only pulled
in on the dedicated settings page. Update the integration assertion
accordingly.
- Return lightweight SimpleNamespace snapshots from the sample_client
and sample_project fixtures so downstream tests can access .id and
.name without re-attaching the SQLAlchemy instance to a new session.
- In test_settings_currency_can_be_changed, persist the Settings row
when get_settings() returns a transient instance, and re-query for
verification instead of refresh() (which fails on transient objects).
WebhookDelivery.next_retry_at is a naive DateTime column. Strip the
tzinfo before passing it to mark_retrying() so the round-trip
comparison after commit/reload does not fail with a naive/aware
mismatch.
Supplier.stock_items is no longer a list-like relationship; assert
against the dynamic supplier_items relationship using .count() and
.first() instead.
With foreign keys enforced in tests, hard-coded client_id=1 and
user_id=1 references no longer work because no such rows exist in the
isolated test database. Create the required parent rows (Client for
inventory project tests, Client + User for the burndown/SavedFilter
smoke tests) before inserting the children.
Introduce a small set_client_portal_access helper that updates the
user row via a single UPDATE and expires the session, replacing the
scattered no_autoflush + safe_commit_with_retry + safe_get_user dance
in every test. Also auto-disable audit logging for this module to
avoid SQLite write-lock contention during portal tests, and fix a few
indentation bugs where assertions sat outside the app_context block.
- Turn on PRAGMA foreign_keys=ON for every SQLite connection so
ondelete="CASCADE" and other FK constraints are exercised by tests.
- Disable FK enforcement only for DROP TABLE statements, since the
schema has cyclic references (deals/leads/projects/quotes) and
drop_all() cannot order them cleanly.
- Seed admin/user/manager/subcontractor roles in the app fixture so
route tests that validate against the role table no longer need to
run the full permission seed command.
- Make TimeEntryFactory.end_time deterministic relative to start_time
so created entries always represent a valid 2h window.
The legacy api.py module exposes _get_client_id_from_session, which is
imported by client-portal tests via app.routes.api. Surface it through
the package __init__ so the import works regardless of which loader
path is taken.
Use request.get_json(silent=True) so a request with the wrong
Content-Type or a malformed body falls through to the existing
validation path and returns 400 instead of raising a 415/parse error.
Inspecting the engine directly opens a second connection, which under
SQLite can contend with a write lock held by the current flush and
cause spurious failures during tests. Bind the inspector to the
session's existing connection when available and fall back to the
engine otherwise.
Match the behaviour of the API v1 endpoint by marking the supplier as
inactive instead of hard-deleting it. This preserves referential
integrity with purchase orders, stock items and audit data and removes
the "cannot delete supplier with associated stock items" failure mode.
Add multi-select with Shift/Ctrl toggles and marquee selection on the invoice
and quote PDF template editors, plus a shared transformer, group nudge with
snap-aware arrow keys, multi copy/paste/duplicate, align-to-page or
align-to-selection, horizontal/vertical distribute, and Ctrl/Cmd+G group /
Ctrl/Cmd+Shift+G ungroup.
Expose horizontal (left/center/right/justify) and vertical (top/middle/bottom)
text controls in the properties panel; export verticalAlign, group_id,
locked, and hidden in template JSON where set. Skip hidden elements in
ReportLab rendering; draw justified text on canvas via Paragraph; honor
verticalAlign when a text box height is present.
Introduce app/static/js/pdf_editor helpers (loaded with dynamic import from
the templates), pdf_editor.css, a layers panel with visibility and lock toggles,
smart-guide snapping, and optional ruler chrome. Document behaviour and manual
QA in docs/PDF_LAYOUT_EDITOR.md; add tests/test_pdf_template_schema.py for
optional template fields.
Closes#619.
Security pytest:
- Run an explicit node list instead of -m security over the whole tests
tree, so collection stays small and nothing is spuriously deselected.
- Use a writable pytest cache under INSTALLATION_CONFIG_DIR and filter
the known Flask-SQLAlchemy SAWarning on metadata DROP ordering.
- Add scripts/ci/security-pytest.sh and wire Makefile, run-tests.sh/.bat,
and ci-comprehensive to call it for a single source of truth.
Safety:
- Write JSON to .test_installation_config/safety-report.json (with the
rest of local CI artifacts) instead of the repo root.
- Run scripts/ci/sanitize_safety_report.py after each scan so paths in
the report are workspace-relative for artifacts and reviews.
- Capture Safety exit codes so failures still print where the report was
written; use python -m safety in workflows where appropriate.
Release and legacy workflows pick up the new report path, sanitizer,
and a pinned Safety install where the CLI is invoked.
Safety 3.0.1 crashes on import with Typer >= 0.17 (AttributeError:
typer.rich_utils). Pin the test dependency to 3.7.0 so local CI and
workflows can run the Safety CLI reliably.
Drives ``mypy app/`` from 567 errors in 208 files to 0 errors across the
376 source files checked by ``./scripts/run-ci-local.sh code-quality``.
Configuration & dependencies
- pyproject.toml: enable implicit_optional (Flask-style ``x: str = None``
defaults), silence truthy-function/truthy-bool (legitimate import-guard
checks like ``KanbanColumn``), and disable warn_return_any (SQLAlchemy
1.x ``Query`` API returns Any pervasively). Add module overrides for
``app.models.*``, repositories, base CRUD service, and known
``joinedload`` / ``Query.paginate`` callers where mypy cannot model the
Flask-SQLAlchemy runtime API without a plugin.
- requirements-test.txt: pin ``types-requests``, ``types-bleach``,
``types-Markdown``, ``types-python-dateutil`` so mypy stops complaining
about missing stubs.
Latent bugs fixed while driving mypy to zero
- app/utils/logger.py, app/utils/datetime_utils.py: drop imports of
symbols that don't exist (``get_performance_metrics``,
``from_app_timezone``, ``to_app_timezone``) — these would have raised
at import time on first use.
- app/services/currency_service.py: ``from typing import Decimal`` was a
bug (typing has no Decimal); switch to ``decimal.Decimal`` and rename
the ``D`` alias.
- app/utils/env_validation.py, app/utils/role_migration.py: ``Dict[str,
any]`` → ``Dict[str, Any]`` (built-in ``any`` is not a type).
- app/utils/email.py: introduce ``send_template_email`` and update the
three callers (``client_approval_service``,
``client_notification_service``, ``workflow_engine``) that were
passing ``to=``/``template=``/etc. to ``send_email`` whose signature
doesn't accept them — calls would have raised TypeError at runtime.
- app/services/permission_service.py: rewrite ``grant_permission`` /
``revoke_permission`` to use the actual ``Role`` ↔ ``Permission``
many-to-many relationship; the old code referenced non-existent
``Permission.role_id`` / ``Permission.granted`` columns.
- app/services/gps_tracking_service.py: pass the required ``title`` and
``expense_date`` fields when creating mileage ``Expense`` rows.
- app/services/workflow_engine.py: ``_perform_action`` now forwards the
``rule`` argument to ``_action_log_time``, and ``_action_webhook``
short-circuits when ``url`` is missing.
- app/services/time_tracking_service.py: validate ``start_time`` /
``end_time`` before comparing them.
- app/services/export_service.py: build CSV in a ``StringIO`` then wrap
the bytes in ``BytesIO`` — ``csv.writer`` requires text I/O.
- app/integrations/peppol_smp.py: avoid attribute access on ``None`` in
the SMP ``href`` fallback.
- app/integrations/{github,gitlab,slack}.py: coerce query-string params
to strings so ``requests.get(params=...)`` matches the typed signature
(and is what the HTTP layer expects anyway).
- app/integrations/{xero,quickbooks}.py: guard ``get_access_token()``
returning ``None`` before calling private ``_api_request`` helpers.
Annotation-only changes
- Add ``Dict[str, Any]`` / ``list`` / ``Optional[...]`` annotations to
service dict-literals that mypy could not infer from heterogeneous
values (``ai_suggestion_service``, ``ai_categorization_service``,
``custom_report_service``, ``unpaid_hours_service``,
``integration_service``, ``invoice_service``, ``backup_service``,
``inventory_report_service``, ``analytics_service``, etc.).
- ``app/utils/event_bus.py``: ``emit_event`` accepts ``str |
WebhookEvent`` and normalizes to ``str`` so all call-sites type-check.
- ``app/utils/api_responses.py``: introduce ``ApiResponse`` alias for
``Response | tuple[Response, int] | tuple[str, int]``.
- ``app/utils/budget_forecasting.py``: forecasting helpers return
``Optional[Dict]`` (they already returned ``None`` when the project
was missing).
- ``app/utils/pdf_generator_reportlab.py``: ``_normalize_color`` is
``Optional[str]``.
- ``app/utils/pdfa3.py``: remove invalid ``force_version=None`` retry
call.
- Narrow ``type: ignore`` markers on optional-dependency fallbacks
(``redis``, ``bleach``, ``markdown``, ``babel``,
``powerpoint_export``) and on the documented ``requests.Session``
/ ``RotatingFileHandler`` typeshed limitations.
Pure formatting pass to satisfy ``./scripts/run-ci-local.sh code-quality``:
no behavioural changes, just consistent line wrapping, import ordering,
and trailing-newline normalization across routes, models, services, and
utility modules.
Introduce scripts/run-ci-local.sh and scripts/ci/lib.sh to run the same
checks as ci-comprehensive.yml. Ignore .test_installation_config/ where
the script writes INSTALLATION_CONFIG_DIR by default.
Remove obsolete skips across admin, calendar, invoices, payments, PDF,
uploads, and related tests. Use /login and passwords consistent with
conftest; fix team chat setup in silent-exception tests; assert expense
currency with locale-tolerant amount matching; align admin logo tests
with admin_authenticated_client credentials.
Switch to the listen + http2 on; form for modern nginx. Add Docker DNS
resolver and a variable-based proxy_pass so nginx can start before the
app container registers, and apply the same upstream to /socket.io/.
Wrap create flow in a single try/except with explicit step labels for
logging and diagnostics. Harden logging around current_user and keep
validation/flash behavior while making failures easier to trace.
Skip network OTLP export when TESTING is set unless OTEL_ENABLE_IN_TESTS=1.
Register shutdown once at exit, reset providers cleanly in tests, and
briefly silence OTEL loggers during shutdown to avoid noisy CI output.
Flask-SQLAlchemy exposes a scoped_session proxy; reading _flushing on the
proxy did not reflect the underlying session, so Settings.get_settings()
could run add/commit mid-flush and break tests. Unwrap to the real session
before checking _flushing and _warn_on_events.
Move shell layout fixes from [Unreleased] to CHANGELOG [5.5.5].
Sync BUILD_CONFIGURATION.md version code example and setup.py snippet
with setup.py (single source of truth).
Remove stray closing </div> tags from admin backups, admin API
tokens, and quote detail templates so content and modals stay
inside #mainContent and the footer aligns with the main column.
Drop max-w-7xl from the primary shell; wrap main + attribution in
a flex column so the scroll area fills width beside the sidebar.
Align the support banner inner row to full width.
Documented in CHANGELOG.md under [Unreleased].
Bump package version in setup.py to 5.5.4.
CHANGELOG: document 5.5.4 (full-database restore hardening and backup/restore documentation).
docs/BUILD_CONFIGURATION.md: refresh version-code example and setup.py snippet to 5.5.4 (50504).
Dries Peeters