fix: close remaining task 2 review findings
This commit is contained in:
@@ -8,7 +8,13 @@ from .operational_state import blocker_fingerprint
|
|||||||
|
|
||||||
|
|
||||||
def upsert_blocker(session, tenant_id: int, payload: dict, *, installation_id: int | None = None) -> BlockedProduct:
|
def upsert_blocker(session, tenant_id: int, payload: dict, *, installation_id: int | None = None) -> BlockedProduct:
|
||||||
fingerprint = payload.get("blocker_fingerprint") or blocker_fingerprint(payload.get("tenant", str(tenant_id)), payload.get("company", ""), payload.get("product_code", ""), payload.get("field", ""), payload.get("reason_code", ""), payload.get("input_fingerprint", ""))
|
supplied_fingerprint = payload.get("blocker_fingerprint")
|
||||||
|
if installation_id is None:
|
||||||
|
fingerprint = supplied_fingerprint or blocker_fingerprint(payload.get("tenant", str(tenant_id)), payload.get("company", ""), payload.get("product_code", ""), payload.get("field", ""), payload.get("reason_code", ""), payload.get("input_fingerprint", ""))
|
||||||
|
elif supplied_fingerprint:
|
||||||
|
fingerprint = blocker_fingerprint(supplied_fingerprint, "", "", "", "", "", installation_id=installation_id)
|
||||||
|
else:
|
||||||
|
fingerprint = blocker_fingerprint(payload.get("tenant", str(tenant_id)), payload.get("company", ""), payload.get("product_code", ""), payload.get("field", ""), payload.get("reason_code", ""), payload.get("input_fingerprint", ""), installation_id=installation_id)
|
||||||
row = session.scalar(select(BlockedProduct).where(BlockedProduct.blocker_fingerprint == fingerprint))
|
row = session.scalar(select(BlockedProduct).where(BlockedProduct.blocker_fingerprint == fingerprint))
|
||||||
now = datetime.now(timezone.utc)
|
now = datetime.now(timezone.utc)
|
||||||
if row is None:
|
if row is None:
|
||||||
|
|||||||
@@ -3,8 +3,9 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
from datetime import datetime, timedelta, timezone
|
from datetime import datetime, timedelta, timezone
|
||||||
|
from decimal import Decimal, ROUND_HALF_UP
|
||||||
|
|
||||||
from sqlalchemy import and_, case, func, or_, select
|
from sqlalchemy import Numeric, and_, case, cast, func, or_, select
|
||||||
|
|
||||||
from .config import settings
|
from .config import settings
|
||||||
from .events import list_events
|
from .events import list_events
|
||||||
@@ -46,7 +47,9 @@ def _health_expression(now):
|
|||||||
def _fiscal_completion_expression():
|
def _fiscal_completion_expression():
|
||||||
total = func.coalesce(AgentSnapshot.total_products, 0)
|
total = func.coalesce(AgentSnapshot.total_products, 0)
|
||||||
complete = func.coalesce(AgentSnapshot.fiscally_complete, 0)
|
complete = func.coalesce(AgentSnapshot.fiscally_complete, 0)
|
||||||
raw = case((total > 0, complete * 100.0 / total), else_=func.coalesce(AgentSnapshot.coverage_percent, 0))
|
numeric_total = cast(total, Numeric(18, 6))
|
||||||
|
numeric_complete = cast(complete * 100, Numeric(18, 6))
|
||||||
|
raw = case((total > 0, numeric_complete / numeric_total), else_=cast(func.coalesce(AgentSnapshot.coverage_percent, 0), Numeric(18, 6)))
|
||||||
return func.round(case((raw < 0, 0), (raw > 100, 100), else_=raw), 1)
|
return func.round(case((raw < 0, 0), (raw > 100, 100), else_=raw), 1)
|
||||||
|
|
||||||
|
|
||||||
@@ -54,8 +57,10 @@ def _fiscal_completion(snapshot) -> float:
|
|||||||
total = int(getattr(snapshot, "total_products", 0) or 0)
|
total = int(getattr(snapshot, "total_products", 0) or 0)
|
||||||
complete = int(getattr(snapshot, "fiscally_complete", 0) or 0)
|
complete = int(getattr(snapshot, "fiscally_complete", 0) or 0)
|
||||||
if total:
|
if total:
|
||||||
return round(max(0, min(total, complete)) * 100 / total, 1)
|
value = Decimal(max(0, min(total, complete)) * 100) / Decimal(total)
|
||||||
return round(max(0, min(100, float(getattr(snapshot, "coverage_percent", 0) or 0))), 1)
|
else:
|
||||||
|
value = Decimal(str(max(0, min(100, float(getattr(snapshot, "coverage_percent", 0) or 0)))))
|
||||||
|
return float(value.quantize(Decimal("0.1"), rounding=ROUND_HALF_UP))
|
||||||
|
|
||||||
|
|
||||||
def _fiscal_status(snapshot) -> str:
|
def _fiscal_status(snapshot) -> str:
|
||||||
@@ -357,7 +362,7 @@ def dashboard_summary(session, now=None):
|
|||||||
outdated_query = installation_query.where(version_filter).order_by(Installation.id.asc()).limit(ATTENTION_CANDIDATE_LIMIT)
|
outdated_query = installation_query.where(version_filter).order_by(Installation.id.asc()).limit(ATTENTION_CANDIDATE_LIMIT)
|
||||||
outdated_agents = [installation_view(i, a, t, s, now=now) for i, a, t, s in session.execute(outdated_query).all()]
|
outdated_agents = [installation_view(i, a, t, s, now=now) for i, a, t, s in session.execute(outdated_query).all()]
|
||||||
|
|
||||||
blocker_query = select(BlockedProduct, Tenant, Installation, Agent, AgentSnapshot).join(Tenant, Tenant.id == BlockedProduct.tenant_id).outerjoin(Installation, Installation.id == BlockedProduct.installation_id).outerjoin(Agent, Agent.id == Installation.agent_id).outerjoin(AgentSnapshot, AgentSnapshot.installation_id == Installation.id).where(open_filter, BlockedProduct.field.in_(BLOCKER_FIELDS[:-1])).order_by(BlockedProduct.last_seen_at.desc(), BlockedProduct.id.desc()).limit(ATTENTION_CANDIDATE_LIMIT)
|
blocker_query = select(BlockedProduct, Tenant, Installation, Agent, AgentSnapshot).join(Tenant, Tenant.id == BlockedProduct.tenant_id).outerjoin(Installation, Installation.id == BlockedProduct.installation_id).outerjoin(Agent, Agent.id == Installation.agent_id).outerjoin(AgentSnapshot, AgentSnapshot.installation_id == Installation.id).where(open_filter, func.upper(BlockedProduct.field).in_(BLOCKER_FIELDS[:-1])).order_by(BlockedProduct.last_seen_at.desc(), BlockedProduct.id.desc()).limit(ATTENTION_CANDIDATE_LIMIT)
|
||||||
blocker_candidates = session.execute(blocker_query).all()
|
blocker_candidates = session.execute(blocker_query).all()
|
||||||
|
|
||||||
attention = []
|
attention = []
|
||||||
|
|||||||
@@ -10,8 +10,10 @@ STATES = {
|
|||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
def blocker_fingerprint(tenant: str, company: str, product_code: str, field: str, reason_code: str, input_fingerprint: str) -> str:
|
def blocker_fingerprint(tenant: str, company: str, product_code: str, field: str, reason_code: str, input_fingerprint: str, installation_id: str | int | None = None) -> str:
|
||||||
raw = {"tenant": tenant, "company": company, "product_code": product_code, "field": field, "reason_code": reason_code, "input_fingerprint": input_fingerprint}
|
raw = {"tenant": tenant, "company": company, "product_code": product_code, "field": field, "reason_code": reason_code, "input_fingerprint": input_fingerprint}
|
||||||
|
if installation_id is not None:
|
||||||
|
raw["installation_id"] = str(installation_id)
|
||||||
return hashlib.sha256(json.dumps(raw, sort_keys=True, separators=(",", ":")).encode()).hexdigest()
|
return hashlib.sha256(json.dumps(raw, sort_keys=True, separators=(",", ":")).encode()).hexdigest()
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -2,7 +2,8 @@ from sqlalchemy import create_engine, select
|
|||||||
from sqlalchemy.orm import Session
|
from sqlalchemy.orm import Session
|
||||||
|
|
||||||
from central.app.blocked_products import list_blockers, resolve_product_blockers, retry_due, upsert_blocker
|
from central.app.blocked_products import list_blockers, resolve_product_blockers, retry_due, upsert_blocker
|
||||||
from central.app.models import Agent, Base, BlockedProduct, Tenant
|
from central.app.models import Agent, Base, BlockedProduct, Installation, Tenant
|
||||||
|
from central.app.operational_state import blocker_fingerprint
|
||||||
|
|
||||||
|
|
||||||
def _session():
|
def _session():
|
||||||
@@ -48,3 +49,24 @@ def test_same_input_without_retry_schedule_is_cache_hit():
|
|||||||
|
|
||||||
assert retry_due(row, "v1") is False
|
assert retry_due(row, "v1") is False
|
||||||
assert retry_due(row, "v2") is True
|
assert retry_due(row, "v2") is True
|
||||||
|
|
||||||
|
|
||||||
|
def test_authenticated_installations_get_distinct_fingerprints_for_identical_blockers():
|
||||||
|
session, tenant = _session()
|
||||||
|
agent_a = Agent(tenant_id=tenant.id, name="agent-a", token_hash="agent-a".ljust(64, "x"))
|
||||||
|
agent_b = Agent(tenant_id=tenant.id, name="agent-b", token_hash="agent-b".ljust(64, "x"))
|
||||||
|
session.add_all([agent_a, agent_b])
|
||||||
|
session.flush()
|
||||||
|
installation_a = Installation(agent_id=agent_a.id, tenant_id=tenant.id, installation_id="00000000-0000-4000-8000-000000000201", display_name="Store A")
|
||||||
|
installation_b = Installation(agent_id=agent_b.id, tenant_id=tenant.id, installation_id="00000000-0000-4000-8000-000000000202", display_name="Store B")
|
||||||
|
session.add_all([installation_a, installation_b])
|
||||||
|
session.flush()
|
||||||
|
payload = {"tenant": "t", "company": "c", "product_code": "P1", "field": "CFOP", "reason_code": "MISSING", "input_fingerprint": "v1"}
|
||||||
|
|
||||||
|
first = upsert_blocker(session, tenant.id, payload, installation_id=installation_a.id)
|
||||||
|
second = upsert_blocker(session, tenant.id, payload, installation_id=installation_b.id)
|
||||||
|
|
||||||
|
assert first.id != second.id
|
||||||
|
assert first.blocker_fingerprint != second.blocker_fingerprint
|
||||||
|
assert blocker_fingerprint("t", "c", "P1", "CFOP", "MISSING", "v1") == blocker_fingerprint("t", "c", "P1", "CFOP", "MISSING", "v1")
|
||||||
|
assert blocker_fingerprint("t", "c", "P1", "CFOP", "MISSING", "v1", installation_id=installation_a.id) != blocker_fingerprint("t", "c", "P1", "CFOP", "MISSING", "v1", installation_id=installation_b.id)
|
||||||
|
|||||||
@@ -280,3 +280,33 @@ def test_worst_coverage_sort_uses_the_displayed_derived_percentage():
|
|||||||
assert result["items"][0]["installation_id"] == second.installation_id
|
assert result["items"][0]["installation_id"] == second.installation_id
|
||||||
assert result["items"][0]["fiscal_completion_percent"] == 20
|
assert result["items"][0]["fiscal_completion_percent"] == 20
|
||||||
assert first.installation_id != second.installation_id
|
assert first.installation_id != second.installation_id
|
||||||
|
|
||||||
|
|
||||||
|
def test_worst_coverage_sort_cannot_disagree_with_one_decimal_display():
|
||||||
|
session_factory = setup_db()
|
||||||
|
with session_factory() as session:
|
||||||
|
_, _, first, first_snapshot = _installation(session, 1, coverage=0)
|
||||||
|
_, _, second, second_snapshot = _installation(session, 2, coverage=0)
|
||||||
|
first_snapshot.total_products = 79
|
||||||
|
first_snapshot.fiscally_complete = 5
|
||||||
|
second_snapshot.total_products = 16
|
||||||
|
second_snapshot.fiscally_complete = 1
|
||||||
|
session.commit()
|
||||||
|
|
||||||
|
result = list_installations(session, sort="worst_coverage", page_size=10, now=NOW)
|
||||||
|
displayed = [item["fiscal_completion_percent"] for item in result["items"]]
|
||||||
|
|
||||||
|
assert displayed == sorted(displayed)
|
||||||
|
assert displayed == [6.3, 6.3]
|
||||||
|
assert {item["installation_id"] for item in result["items"]} == {first.installation_id, second.installation_id}
|
||||||
|
|
||||||
|
|
||||||
|
def test_lowercase_required_blocker_field_reaches_attention_candidates():
|
||||||
|
session_factory = setup_db()
|
||||||
|
with session_factory() as session:
|
||||||
|
tenant, _, installation, _ = _installation(session, 1)
|
||||||
|
blocker = _blocker(session, tenant, product="LOWER", field="cfop", index=120, installation_id=installation.id)
|
||||||
|
|
||||||
|
attention = dashboard_summary(session, now=NOW)["attention_items"]
|
||||||
|
|
||||||
|
assert any(item["link"] == f"/blockers/{blocker.id}" for item in attention)
|
||||||
|
|||||||
Reference in New Issue
Block a user