From 7ae6f850324a10165fababd78e50878cf2599dd8 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 8 Oct 2026 15:34:19 -0400 Subject: [PATCH] =?UTF-8?q?fix(settings):=20Notifications=20opens=20on=20f?= =?UTF-8?q?irst=20visit=20=E2=80=94=20no=20second=20begin=20on=20an=20auto?= =?UTF-8?q?begun=20session?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reading the settings autobegins the session's transaction, so the nested db.begin() that saves the first admin's zone as smtp.tz raised InvalidRequestError and every visit 500'd until a zone was saved. The integration test signed in without a zone, so that path never ran; it now has a test of its own. Co-Authored-By: Claude Opus 5.5 --- steward/settings/routes.py | 5 +++-- tests/integration/test_users_page.py | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/steward/settings/routes.py b/steward/settings/routes.py index 8afff9a..c905c68 100644 --- a/steward/settings/routes.py +++ b/steward/settings/routes.py @@ -222,8 +222,9 @@ async def notifications(): settings = await get_all_settings(db) if not settings["smtp.tz"] and session.get("tz"): # "Also send to" reads times in the first admin's zone until one is chosen. - async with db.begin(): - await set_setting(db, "smtp.tz", session["tz"]) + # Reading the settings already began this session's transaction. + await set_setting(db, "smtp.tz", session["tz"]) + await db.commit() settings["smtp.tz"] = session["tz"] await _reload_app_config() who = await audience(db) diff --git a/tests/integration/test_users_page.py b/tests/integration/test_users_page.py index 2395ff0..f57b87a 100644 --- a/tests/integration/test_users_page.py +++ b/tests/integration/test_users_page.py @@ -198,3 +198,24 @@ def test_an_account_sets_its_own_address_and_switches_and_an_admin_can_too(app): assert "Who gets email" in page and 'name="notify.alerts.viewer"' in page and "Also send to" in page asyncio.run(_go()) + + +def test_the_first_admin_to_open_notifications_sets_the_also_send_to_zone(app): + async def _go(): + from steward.core.settings import get_setting, set_setting + + async def _tz(): + async with app.db_sessionmaker() as s: + return await get_setting(s, "smtp.tz") + + async with app.db_sessionmaker() as s: + async with s.begin(): + await set_setting(s, "smtp.tz", "") + for zone in ("America/Chicago", "Asia/Tokyo"): + admin, _ = await _client(app) + async with admin.session_transaction() as sess: + sess["tz"] = zone + assert (await admin.get("/settings/notifications/")).status_code == 200 + assert await _tz() == "America/Chicago" # the second admin's zone changes nothing + + asyncio.run(_go())