diff --git a/app/routes/tenant_settings.py b/app/routes/tenant_settings.py index b03df9a..c3c77fc 100644 --- a/app/routes/tenant_settings.py +++ b/app/routes/tenant_settings.py @@ -219,20 +219,32 @@ def plan(): 'allow_custom_domain': tenant.allow_custom_domain, } - # Live counts (tenant DB) + # Live counts (tenant DB), one axis at a time. + # + # These used to be four calls inside a single dict literal in one + # try/except. Python evaluates every value before assigning, so ONE + # failing counter discarded the whole dict and the page rendered 0 + # for all four axes — including users and facilities, which were + # fine. That is exactly how a wrong column name in the issues + # counter presented as "no usage number ever updates". from app.tenancy.quota import ( count_active_users, count_active_facilities, count_inspections_this_month, count_issues_this_month, ) - try: - quota_usage = { - 'users': count_active_users(), - 'facilities': count_active_facilities(), - 'inspections': count_inspections_this_month(), - 'issues': count_issues_this_month(), - } - except Exception as exc: - logger.error('tenant_settings.plan: quota count failed: %s', exc) + for axis, counter in ( + ('users', count_active_users), + ('facilities', count_active_facilities), + ('inspections', count_inspections_this_month), + ('issues', count_issues_this_month), + ): + try: + quota_usage[axis] = counter() + except Exception as exc: + # None (not 0) so the page shows "—": an unknown count and + # a genuine zero must not look the same. + quota_usage[axis] = None + logger.error('tenant_settings.plan: %s count failed: %s', + axis, exc) # MT-8: billing fields are already on g.tenant — no extra DB query needed. billing_enabled = current_app.config.get('BILLING_ENABLED', False) diff --git a/app/templates/tenant_settings/plan.html b/app/templates/tenant_settings/plan.html index 1c3822e..e58602f 100644 --- a/app/templates/tenant_settings/plan.html +++ b/app/templates/tenant_settings/plan.html @@ -98,17 +98,22 @@ ('facilities', 'Active Facilities (total)', plan_info.max_facilities), ] %} {% for key, label, limit in axes %} - {% set current = quota_usage.get(key, 0) %} - {% set pct = ((current / limit * 100) | int) if limit else 0 %} - {% set over = limit and current >= limit %} + {# None = the counter failed (logged server-side). Rendered as "—" + rather than 0 so an unknown count is never mistaken for real usage. #} + {% set current = quota_usage.get(key) %} + {% set unknown = current is none %} + {% set pct = ((current / limit * 100) | int) if (limit and not unknown) else 0 %} + {% set over = limit and not unknown and current >= limit %}
{{ label }} - {{ current }}{% if limit %} / {{ limit }}{% else %} / unlimited{% endif %} + {% if unknown %} + — + {% else %}{{ current }}{% endif %}{% if limit %} / {{ limit }}{% else %} / unlimited{% endif %}
- {% if limit %} + {% if limit and not unknown %}
diff --git a/app/tenancy/quota.py b/app/tenancy/quota.py index aa8640a..5fcb5d7 100644 --- a/app/tenancy/quota.py +++ b/app/tenancy/quota.py @@ -9,7 +9,7 @@ no second control-DB round-trip is needed. Quota axes: inspections → Inspection.inspection_date in current month, status='completed' - issues → Issue.created_at in current month + issues → Issue.reported_at in current month users → User.active == True (total, not monthly) facilities → Facility.active == True (total, not monthly) @@ -18,10 +18,11 @@ all quota checks pass, single-tenant behaviour unchanged. """ import logging -from datetime import datetime from flask import g, current_app +from app.utils.time_utils import now_eastern + logger = logging.getLogger(__name__) @@ -30,7 +31,15 @@ def _mt_enabled(): def _month_window(): - now = datetime.now() + """[start, end) of the current month, in the timezone the rows are stamped in. + + now_eastern(), not datetime.now(): every timestamp in the tenant DB is + written by now_eastern() (rule 2). On a UTC server the two differ by 4-5 + hours, so a plain now() puts the month boundary in the wrong place and the + first hours of each month count the wrong rows — a discrepancy that only + appears on the 1st and is gone before anyone investigates it. + """ + now = now_eastern() start = now.replace(day=1, hour=0, minute=0, second=0, microsecond=0) if now.month == 12: end = now.replace(year=now.year + 1, month=1, day=1, @@ -52,11 +61,20 @@ def count_inspections_this_month(): def count_issues_this_month(): + """Issues filed this month. + + `reported_at`, NOT `created_at` — the issues table has no created_at + column. (IssueComment does, in the same module, which is how the wrong name + got here.) Referencing a missing column raises AttributeError while the + query is built, and every caller wraps this in a try/except, so the failure + was invisible: the plan page silently showed 0 for EVERY axis and the + issues quota was never evaluated at all. + """ from app.models.issue import Issue start, end = _month_window() return (Issue.query - .filter(Issue.created_at >= start, - Issue.created_at < end) + .filter(Issue.reported_at >= start, + Issue.reported_at < end) .count()) diff --git a/tests/test_quota_counts.py b/tests/test_quota_counts.py new file mode 100644 index 0000000..c7d28e1 --- /dev/null +++ b/tests/test_quota_counts.py @@ -0,0 +1,156 @@ +""" +tests/test_quota_counts.py +--------------------------- +Regression guards for the live quota counters in app/tenancy/quota.py. + +Why these exist +--------------- +`count_issues_this_month()` filtered on `Issue.created_at`, a column the issues +table does not have (the timestamp is `reported_at`; `created_at` belongs to +IssueComment, declared in the same module). Referencing a missing mapped +attribute raises AttributeError while the query is being BUILT — and every +caller wraps the counters in try/except, so the failure was completely silent: + + * `/settings/plan` evaluated all four counters inside one dict literal, so the + single failure discarded the whole dict and the page rendered 0 for EVERY + axis — including users and facilities, which were working. It read as + "usage numbers never update". + * `@quota_soft_check('issues')` swallowed the same exception and returned + None, so the issues quota was never evaluated for any tenant. + +A counter that returns a wrong number is visible. A counter that raises behind +a try/except is not — which is why these assert the counters actually COUNT, +rather than merely that they do not raise. + +The counters are plain queries against the bound session; they do not gate on +MULTI_TENANT_ENABLED, so they can be called directly on the single-tenant test +fixture. +""" + +import pytest + + +@pytest.fixture +def client(app): + with app.app_context(): + from app import db + db.drop_all() + db.create_all() + yield app.test_client() + db.session.remove() + + +def _facility(name='Main Office', active=True): + from app import db + from app.models.facility import Facility + fac = Facility(name=name, active=active) + db.session.add(fac) + db.session.commit() + return fac + + +def _user(username, role='inspector', active=True): + from app import db + from app.models.user import User + u = User(username=username, full_name=username.title(), role=role, + email=f'{username}@example.com', active=active, password_set=True) + u.set_password('pw-correct1') + db.session.add(u) + db.session.commit() + return u + + +# ── issues ─────────────────────────────────────────────────────────────────── + +def test_issue_counter_counts_issues_reported_this_month(client): + """The regression: this raised AttributeError instead of returning 1.""" + from app import db + from app.models.issue import Issue + from app.tenancy.quota import count_issues_this_month + + fac = _facility() + assert count_issues_this_month() == 0 + + db.session.add(Issue(facility_id=fac.id, severity='high', + description='Leaking faucet', status='open')) + db.session.commit() + + assert count_issues_this_month() == 1 + + +def test_issue_counter_excludes_a_previous_month(client): + """Proves the month window is applied to the right column, not just that + the query runs.""" + from datetime import timedelta + from app import db + from app.models.issue import Issue + from app.tenancy.quota import count_issues_this_month, _month_window + + fac = _facility() + start, _ = _month_window() + + db.session.add(Issue(facility_id=fac.id, severity='low', description='Old', + status='open', reported_at=start - timedelta(days=1))) + db.session.add(Issue(facility_id=fac.id, severity='low', description='New', + status='open')) + db.session.commit() + + assert count_issues_this_month() == 1 + + +# ── inspections ────────────────────────────────────────────────────────────── + +def test_inspection_counter_counts_only_completed(client): + from app import db + from app.models.inspection import Inspection, InspectionTemplate + from app.tenancy.quota import count_inspections_this_month + + fac = _facility() + insp = _user('ivy') + tmpl = InspectionTemplate(name='Restroom Check', active=True, form_schema=[]) + db.session.add(tmpl) + db.session.commit() + + db.session.add(Inspection(template_id=tmpl.id, facility_id=fac.id, + inspector_id=insp.id, status='completed')) + db.session.add(Inspection(template_id=tmpl.id, facility_id=fac.id, + inspector_id=insp.id, status='in_progress')) + db.session.commit() + + assert count_inspections_this_month() == 1 + + +# ── users / facilities ─────────────────────────────────────────────────────── + +def test_user_and_facility_counters_count_active_rows_only(client): + from app.tenancy.quota import count_active_users, count_active_facilities + + _user('ada', 'admin') + _user('gone', 'inspector', active=False) + _facility('Live Site') + _facility('Closed Site', active=False) + + assert count_active_users() == 1 + assert count_active_facilities() == 1 + + +# ── the window itself ──────────────────────────────────────────────────────── + +def test_month_window_is_eastern_and_brackets_now(client): + """Rule 2: rows are stamped with now_eastern(), so the window must be too. + + A UTC-clock window on an Eastern-stamped table misplaces the month boundary + by 4-5 hours — wrong only on the 1st, and self-healing before anyone looks. + """ + from app.tenancy.quota import _month_window + from app.utils.time_utils import now_eastern + + start, end = _month_window() + now = now_eastern() + + assert start.tzinfo is None and end.tzinfo is None + assert start <= now < end + assert (start.day, start.hour, start.minute, start.second) == (1, 0, 0, 0) + assert end.day == 1 + assert (end.year, end.month) == ((now.year + 1, 1) if now.month == 12 + else (now.year, now.month + 1))