Aug 21 - Fix plan usage counter

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,
             )
+            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 = {
-                    'users':       count_active_users(),
-                    'facilities':  count_active_facilities(),
-                    'inspections': count_inspections_this_month(),
-                    'issues':      count_issues_this_month(),
-                }
+                    quota_usage[axis] = counter()
                 except Exception as exc:
-                logger.error('tenant_settings.plan: quota count failed: %s', 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)

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 %}
         <div class="mb-3">
           <div class="d-flex justify-content-between mb-1" style="font-size:.85rem;">
             <span>{{ label }}</span>
             <span class="{{ 'text-danger fw-semibold' if over else 'text-muted' }}">
-              {{ current }}{% if limit %} / {{ limit }}{% else %} / <em>unlimited</em>{% endif %}
+              {% if unknown %}
+              <span title="This count could not be read — see the server log.">&mdash;</span>
+              {% else %}{{ current }}{% endif %}{% if limit %} / {{ limit }}{% else %} / <em>unlimited</em>{% endif %}
             </span>
           </div>
-          {% if limit %}
+          {% if limit and not unknown %}
           <div class="progress" style="height:6px;">
             <div class="progress-bar {{ 'bg-danger' if over else ('bg-warning' if pct >= 80 else 'bg-success') }}"
                  role="progressbar" style="width:{{ [pct,100]|min }}%"></div>

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())
 
 

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))