Develop

.github/workflows/ci-comprehensive.yml

@@ -99,6 +99,8 @@ jobs:
         run: |
           if [ "${{ matrix.test-group }}" == "api" ]; then
             pytest -m "api and integration" -v -n auto --cov=app --cov-report=xml --cov-report=html --cov-report=term-missing
+          elif [ "${{ matrix.test-group }}" == "routes" ]; then
+            pytest -m "unit and routes" -v -n 0 --cov=app --cov-report=xml --cov-report=html --cov-report=term-missing
           else
             pytest -m "unit and ${{ matrix.test-group }}" -v -n auto --cov=app --cov-report=xml --cov-report=html --cov-report=term-missing
           fi

app/models/client_note.py

@@ -26,7 +26,10 @@ class ClientNote(db.Model):
 
     # Relationships
     author = db.relationship("User", backref="client_notes")
-    client = db.relationship("Client", backref=db.backref("notes", lazy="dynamic"), passive_deletes=True)
+    client = db.relationship(
+        "Client",
+        backref=db.backref("notes", lazy="dynamic", cascade="all, delete-orphan", passive_deletes=True),
+    )
 
     def __init__(self, content, user_id, client_id, is_important=False):
         """Create a client note.

app/routes/custom_field_definitions.py

@@ -166,9 +166,10 @@ def delete_custom_field_definition(definition_id):
             flash(_("Could not remove custom field from clients due to a database error."), "error")
             return redirect(url_for("custom_field_definitions.list_custom_field_definitions"))
 
-    # Now delete the definition
+    # Now delete the definition (capture id before delete to avoid using detached object)
+    definition_id = definition.id
     db.session.delete(definition)
-    if not safe_commit("delete_custom_field_definition", {"definition_id": definition.id}):
+    if not safe_commit("delete_custom_field_definition", {"definition_id": definition_id}):
         flash(_("Could not delete custom field definition due to a database error."), "error")
     else:
         if client_count > 0:

tests/test_client_note_model.py

@@ -111,7 +111,7 @@ def test_client_has_notes_relationship(app, user, test_client):
         db.session.commit()
 
         db.session.refresh(client)
-        assert len(client.notes) == 2
+        assert client.notes.count() == 2
 
 
 @pytest.mark.unit

tests/test_client_portal.py

@@ -267,14 +267,16 @@ class TestClientPortalRoutes:
     """Test client portal routes"""
 
     def test_client_portal_dashboard_requires_access(self, app, client, user):
-        """Test dashboard requires client portal access"""
+        """Test dashboard requires client portal access - redirects to client portal login when user has no portal access"""
         with app.app_context():
             # Login user without portal access
             with client.session_transaction() as sess:
                 sess["_user_id"] = str(user.id)
 
             response = client.get("/client-portal/dashboard")
-            assert response.status_code == 403
+            assert response.status_code == 302
+            # Client portal 403 handler redirects authenticated non-portal users to client portal login
+            assert "client-portal" in (response.location or "") and "login" in (response.location or "")
 
     def test_client_portal_dashboard_with_access(self, app, client, user, test_client):
         """Test dashboard accessible with portal access"""

tests/test_currency_display.py

@@ -13,6 +13,7 @@
 from app.models import User, Project, Settings, Client, Payment, Invoice, Expense
 from factories import ClientFactory, ProjectFactory, InvoiceFactory, ExpenseFactory
 from flask_login import login_user
+from sqlalchemy import text
 from sqlalchemy.pool import StaticPool
 
 
@@ -108,13 +109,16 @@ def app():
                         except Exception:
                             pass
 
-        try:
-            db.session.execute("PRAGMA journal_mode=WAL;")
-            db.session.execute("PRAGMA synchronous=NORMAL;")
-            db.session.execute("PRAGMA busy_timeout=30000;")
-            db.session.commit()
-        except Exception:
-            db.session.rollback()
+        # PRAGMAs only for file-based SQLite; in-memory (sqlite://) can leave the session broken
+        db_uri = app.config.get("SQLALCHEMY_DATABASE_URI", "") or ""
+        if db_uri.strip().startswith("sqlite:///") and db_uri != "sqlite://":
+            try:
+                db.session.execute(text("PRAGMA journal_mode=WAL;"))
+                db.session.execute(text("PRAGMA synchronous=NORMAL;"))
+                db.session.execute(text("PRAGMA busy_timeout=30000;"))
+                db.session.commit()
+            except Exception:
+                db.session.rollback()
         try:
             yield app
         finally:

tests/test_custom_field_definitions.py

@@ -72,8 +72,9 @@ def test_count_clients_with_value_with_clients(app, admin_user, test_client):
         db.session.add(definition)
         db.session.commit()
 
-        # Set custom field value for client
-        test_client.set_custom_field("test_field", "test_value")
+        # Re-query client so it's in the current session
+        client = Client.query.get(test_client.id)
+        client.set_custom_field("test_field", "test_value")
         db.session.commit()
 
         count = definition.count_clients_with_value()
@@ -102,22 +103,24 @@ def test_count_clients_with_value_ignores_empty(app, admin_user, test_client):
         db.session.add(definition)
         db.session.commit()
 
+        # Re-query client so it's in the current session
+        client = Client.query.get(test_client.id)
         # Set empty value
-        test_client.set_custom_field("test_field", "")
+        client.set_custom_field("test_field", "")
         db.session.commit()
 
         count = definition.count_clients_with_value()
         assert count == 0
 
         # Set whitespace-only value
-        test_client.set_custom_field("test_field", "   ")
+        client.set_custom_field("test_field", "   ")
         db.session.commit()
 
         count = definition.count_clients_with_value()
         assert count == 0
 
         # Set actual value
-        test_client.set_custom_field("test_field", "valid_value")
+        client.set_custom_field("test_field", "valid_value")
         db.session.commit()
 
         count = definition.count_clients_with_value()
@@ -137,15 +140,17 @@ def test_count_clients_with_value_ignores_other_fields(app, admin_user, test_cli
         db.session.add(definition)
         db.session.commit()
 
+        # Re-query client so it's in the current session
+        client = Client.query.get(test_client.id)
         # Set a different field
-        test_client.set_custom_field("other_field", "value")
+        client.set_custom_field("other_field", "value")
         db.session.commit()
 
         count = definition.count_clients_with_value()
         assert count == 0
 
         # Set the correct field
-        test_client.set_custom_field("test_field", "value")
+        client.set_custom_field("test_field", "value")
         db.session.commit()
 
         count = definition.count_clients_with_value()
@@ -190,10 +195,10 @@ def test_delete_custom_field_removes_from_clients(app, admin_user, test_client,
         deleted_definition = CustomFieldDefinition.query.get(definition.id)
         assert deleted_definition is None
 
-        # Verify field is removed from client
-        db.session.refresh(test_client)
-        assert test_client.get_custom_field("debtor_number") is None
-        assert "debtor_number" not in (test_client.custom_fields or {})
+        # Verify field is removed from client (re-query; test_client may be detached after request)
+        client_after = Client.query.get(test_client.id)
+        assert client_after.get_custom_field("debtor_number") is None
+        assert "debtor_number" not in (client_after.custom_fields or {})
 
 
 @pytest.mark.integration
@@ -297,8 +302,8 @@ def test_delete_custom_field_preserves_other_fields(app, admin_user, test_client
 
         assert response.status_code == 200
 
-        # Verify field1 is removed but field2 remains
-        db.session.refresh(test_client)
-        assert test_client.get_custom_field("field1") is None
-        assert test_client.get_custom_field("field2") == "value2"
+        # Verify field1 is removed but field2 remains (re-query; test_client may be detached after request)
+        client_after = Client.query.get(test_client.id)
+        assert client_after.get_custom_field("field1") is None
+        assert client_after.get_custom_field("field2") == "value2"
 

tests/test_oidc_logout.py

@@ -79,6 +79,7 @@ def test_logout_with_post_logout_uri_config(oidc_authenticated_client, app):
     logout attempts RP-Initiated Logout at the provider.
     """
     with app.app_context():
+        app.config["AUTH_METHOD"] = "oidc"
         # Mock oauth client and Config
         with patch("app.routes.auth.oauth") as mock_oauth, patch("app.routes.auth.Config") as mock_config:
             # Configure OIDC with post-logout redirect
@@ -108,6 +109,7 @@ def test_logout_oidc_provider_has_revocation_endpoint_only(oidc_authenticated_cl
     Should use revocation_endpoint as fallback when post_logout URI is configured.
     """
     with app.app_context():
+        app.config["AUTH_METHOD"] = "oidc"
         with patch("app.routes.auth.oauth") as mock_oauth, patch("app.routes.auth.Config") as mock_config:
             mock_config.AUTH_METHOD = "oidc"
             mock_config.OIDC_POST_LOGOUT_REDIRECT_URI = "https://app.example.com/"

tests/test_permissions_routes.py

@@ -132,10 +132,9 @@ def test_delete_role_flow(app, client, admin_user):
     response = client.post(f"/admin/roles/{role_id}/delete", follow_redirects=True)
     assert response.status_code == 200
 
-    # Verify deletion
+    # Verify deletion (assert by name; redirect may trigger sync_permissions_and_roles and reuse id)
     with app.app_context():
-        role = Role.query.get(role_id)
-        assert role is None
+        assert Role.query.filter_by(name="deletable_role").first() is None
 
 
 @pytest.mark.integration

tests/test_uploads_persistence.py

@@ -44,7 +44,7 @@ def authenticated_admin_client(client, admin_user):
     except Exception:
         csrf_enabled = False
 
-    login_data = {"username": admin_user.username}
+    login_data = {"username": admin_user.username, "password": "testpass123"}
     headers = {}
 
     if csrf_enabled: