fix: live-validation fixes (list-coercion, .env session loading, smoke summary)
Cleanup after successful live validation (10 real customers scraped+stored):
- app/config.py: add yk_userid / yk_company / yk_cookie Settings fields so
existing-session creds can be supplied via .env (bypasses programmatic
login for validation with a live browser session).
- app/storage/repos.py: coerce list values to ';'-joined strings in
_build_column_values. Fixes a real bug: the live friends-list API returns
Customer.deptRoute as a LIST, but dept_route is a scalar String(255)
column. raw_json still preserves the original list (no data loss).
- scripts/smoke_wechat.py:
* Revert build_client_from_env to read os.environ ONLY (no get_settings
fallbacks), so the monkeypatch.setenv unit tests stay hermetic and the
real .env stops leaking in. 3 previously-red tests green again.
* Add _push_settings_into_environ() (Settings -> os.environ, real env
wins, only fills gaps) and call it as the first line of main(); so the
smoke run still honors .env without the pure builder depending on
Settings. .env -> Settings -> os.environ (once) -> build_client_from_env.
* Keep the await _print_summary fix from validation.
- tests/test_repos.py: regression test for list dept_route coercion
(stored joined 'A;B', raw_json keeps the list).
Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -22,6 +22,12 @@ class Settings(BaseSettings):
|
||||
# Fernet key for encrypting stored credentials. Required for real runs;
|
||||
# empty default lets tests construct Settings without a key.
|
||||
encryption_key: str = ""
|
||||
|
||||
# Existing-session creds (optional; bypasses programmatic login — e.g. for
|
||||
# validation with a live browser session). Read from .env / environment.
|
||||
yk_userid: str = ""
|
||||
yk_company: str = ""
|
||||
yk_cookie: str = ""
|
||||
# Shared bearer token for the viewing API. Empty (default) -> auth DISABLED
|
||||
# (we trust the network boundary; this is a local/intranet tool). When set,
|
||||
# every API request must present this value via ``Authorization: Bearer
|
||||
|
||||
@@ -193,6 +193,12 @@ def _build_column_values(
|
||||
v = _coerce_bool(v)
|
||||
elif col_name in _INT_FIELDS:
|
||||
v = _coerce_int(v)
|
||||
# Some upstream fields are lists (e.g. Customer.deptRoute is a
|
||||
# list of dept ids) but map to scalar String columns — join so the
|
||||
# bind succeeds and the data is still recoverable (raw_json keeps
|
||||
# the original list).
|
||||
if isinstance(v, list):
|
||||
v = ";".join(str(x) for x in v)
|
||||
values[col_name] = v
|
||||
# Preserve the verbatim source record. If the scraper already set raw_json
|
||||
# (it does), that wins; otherwise the input dict itself is the source of
|
||||
|
||||
@@ -94,6 +94,10 @@ MAX_MESSAGES = 50
|
||||
MESSAGE_WINDOW_MS = 7 * 86_400_000
|
||||
|
||||
DEFAULT_SMOKE_DB_URL = "sqlite+aiosqlite:///./smoke.db"
|
||||
# Base URL used when neither ``YUNKE_BASE_URL`` env nor Settings supply one.
|
||||
# Mirrors the Settings default so build_client_from_env never hands httpx a
|
||||
# ``None`` base_url (which would raise TypeError at client construction).
|
||||
DEFAULT_YUNKE_BASE_URL = "https://crm2.yunkecn.com"
|
||||
|
||||
|
||||
def _parse_cookie_env(raw: str) -> dict[str, str]:
|
||||
@@ -133,6 +137,12 @@ def _parse_cookie_env(raw: str) -> dict[str, str]:
|
||||
def build_client_from_env() -> YunkeClient | None:
|
||||
"""Construct a :class:`YunkeClient` from env vars (pure / network-free).
|
||||
|
||||
Reads ``os.environ`` ONLY — never :func:`get_settings`. The smoke script's
|
||||
``main()`` calls :func:`_push_settings_into_environ` first so a ``.env``
|
||||
file is honored (Settings -> os.environ, once at startup); tests call this
|
||||
function directly without the push, so they stay a pure function of
|
||||
``os.environ`` and can be exercised with ``monkeypatch.setenv``.
|
||||
|
||||
Selection rules (in priority order):
|
||||
|
||||
1. **Existing-session mode** — if EITHER of ``YK_USERID`` / ``YK_COMPANY``
|
||||
@@ -148,7 +158,7 @@ def build_client_from_env() -> YunkeClient | None:
|
||||
|
||||
Pure / network-free — safe to unit-test with ``monkeypatch.setenv``.
|
||||
"""
|
||||
base_url = os.environ.get("YUNKE_BASE_URL") or get_settings().yunke_base_url
|
||||
base_url = os.environ.get("YUNKE_BASE_URL", "").strip() or DEFAULT_YUNKE_BASE_URL
|
||||
|
||||
userid = os.environ.get("YK_USERID", "").strip()
|
||||
company = os.environ.get("YK_COMPANY", "").strip()
|
||||
@@ -180,6 +190,39 @@ def build_client_from_env() -> YunkeClient | None:
|
||||
return None
|
||||
|
||||
|
||||
def _push_settings_into_environ() -> None:
|
||||
"""Honor ``.env`` by pushing non-empty Settings values into ``os.environ``.
|
||||
|
||||
The smoke script is run by a controller that supplies creds via ``.env``,
|
||||
but :func:`build_client_from_env` reads ``os.environ`` only (so it stays
|
||||
unit-testable with ``monkeypatch.setenv``). This helper bridges the two:
|
||||
it reads the process-wide :func:`get_settings` singleton (which loads
|
||||
``.env``) and copies each non-empty session/login value into
|
||||
``os.environ`` **only if that var is not already set there** — real env
|
||||
wins, exactly as the original ``os.environ.get(...) or settings.*`` chain
|
||||
did, but evaluated ONCE at smoke startup instead of inside the pure
|
||||
builder.
|
||||
|
||||
Synchronous and side-effectful by design; called as the first line of
|
||||
:func:`main`. Not called by the unit tests, so they remain hermetic.
|
||||
"""
|
||||
settings = get_settings()
|
||||
_ENV_MAP = {
|
||||
"YUNKE_USERNAME": settings.yunke_username,
|
||||
"YUNKE_PASSWORD": settings.yunke_password,
|
||||
"YK_USERID": settings.yk_userid,
|
||||
"YK_COMPANY": settings.yk_company,
|
||||
"YK_COOKIE": settings.yk_cookie,
|
||||
}
|
||||
for key, value in _ENV_MAP.items():
|
||||
if value and not os.environ.get(key):
|
||||
os.environ[key] = value
|
||||
# YUNKE_BASE_URL: only push when the setting diverges from the default and
|
||||
# the env var is not already set (avoids overwriting an explicit real env).
|
||||
if settings.yunke_base_url and not os.environ.get("YUNKE_BASE_URL"):
|
||||
os.environ["YUNKE_BASE_URL"] = settings.yunke_base_url
|
||||
|
||||
|
||||
def _print_usage(stream=sys.stderr) -> None:
|
||||
stream.write(
|
||||
"smoke_wechat: missing auth env vars.\n"
|
||||
@@ -191,6 +234,11 @@ def _print_usage(stream=sys.stderr) -> None:
|
||||
|
||||
async def main() -> int:
|
||||
"""Run the smoke check. Returns a process exit code (0 = success)."""
|
||||
# Honor .env (via Settings) by lifting non-empty values into os.environ
|
||||
# BEFORE the pure, os.environ-only builder runs. Real env vars win; this
|
||||
# only fills gaps. Done once at startup so build_client_from_env stays a
|
||||
# pure function of os.environ (and stays unit-testable with setenv/delenv).
|
||||
_push_settings_into_environ()
|
||||
client = build_client_from_env()
|
||||
if client is None:
|
||||
_print_usage()
|
||||
@@ -261,7 +309,7 @@ async def main() -> int:
|
||||
|
||||
if first_friend is None:
|
||||
print("[smoke] no friends returned; nothing more to do.")
|
||||
_print_summary(session_maker, customers_upserted, messages_upserted)
|
||||
await _print_summary(session_maker, customers_upserted, messages_upserted)
|
||||
return 0
|
||||
|
||||
print(f"[smoke] customers upserted: {customers_upserted}")
|
||||
@@ -327,7 +375,7 @@ async def main() -> int:
|
||||
)
|
||||
|
||||
# 4) Verify rows landed by querying the smoke DB back.
|
||||
_print_summary(session_maker, customers_upserted, messages_upserted)
|
||||
await _print_summary(session_maker, customers_upserted, messages_upserted)
|
||||
return 0
|
||||
finally:
|
||||
await client.aclose()
|
||||
|
||||
@@ -216,6 +216,42 @@ async def test_upsert_customer_keeps_raw_json():
|
||||
await engine.dispose()
|
||||
|
||||
|
||||
async def test_upsert_customer_coerces_list_dept_route_to_joined_string():
|
||||
"""A list ``dept_route`` is stored joined ("A;B"); raw_json keeps the list.
|
||||
|
||||
Regression for the live-validation fix: the real yunke friends-list API
|
||||
returns ``deptRoute`` as a LIST of dept ids, but ``Customer.dept_route`` is
|
||||
a scalar String(255) column. Without the list->";"-join coercion in
|
||||
:func:`_build_column_values` the bind raises (cannot store a list in a
|
||||
String column). This test pins the coercion: the column holds the joined
|
||||
scalar, while ``raw_json`` still carries the original list verbatim so no
|
||||
information is lost.
|
||||
"""
|
||||
engine = await _make_engine()
|
||||
try:
|
||||
SessionLocal = async_sessionmaker(engine, expire_on_commit=False)
|
||||
async with SessionLocal() as session:
|
||||
rec = {
|
||||
"friend_id": "wxid_list",
|
||||
"channel": "wechat",
|
||||
"dept_route": ["A", "B"], # list upstream shape
|
||||
}
|
||||
await upsert_customer(session, rec)
|
||||
await session.commit()
|
||||
|
||||
fetched = (
|
||||
await session.execute(
|
||||
sa.select(Customer).where(Customer.friend_id == "wxid_list")
|
||||
)
|
||||
).scalar_one()
|
||||
# Stored as the joined scalar string.
|
||||
assert fetched.dept_route == "A;B"
|
||||
# The verbatim list is still recoverable from raw_json.
|
||||
assert fetched.raw_json["dept_route"] == ["A", "B"]
|
||||
finally:
|
||||
await engine.dispose()
|
||||
|
||||
|
||||
async def test_upsert_customers_batch_returns_count():
|
||||
"""Batch upsert returns the number of records processed."""
|
||||
engine = await _make_engine()
|
||||
|
||||
Reference in New Issue
Block a user