Aug 17 - Fix customer inspector issue comment internal banner

This commit is contained in:
2026-08-17 17:01:40 -04:00
parent 98c0dcb7cc
commit 396b774216
2 changed files with 37 additions and 4 deletions
+8 -1
View File
@@ -1376,7 +1376,14 @@ Rendered in `dashboard.html` for `current_user.role == 'customer'`. Uses a Boots
- Each bubble shows: colored avatar circle (color keyed to `author.id % 7`), display name, role badge (Staff / Customer), `is_customer_visible` badge (staff-only view), status-at-time badge, timestamp, and body.
- **Staff commenting:** A hidden checkbox `name="is_customer_visible"` in the Add Comment form defaults to unchecked (staff-only). Checking it marks the comment visible to customers.
- **Customer commenting:** Only shown when `can_customer_comment = is_following or issue.reported_by == current_user.id`. Customer POST bypasses `IssueUpdateForm`; the route sets `is_customer_visible=True` unconditionally.
- **Read filtering:** `GET issues/view` passes `filter_by(is_customer_visible=True)` to customers; staff receive all comments. **Gated by `COMMENTS_VISIBLE_TO_ALL` (temporary, Aug 2026)** — while that config is true the filter is skipped entirely and customers see every comment. The route passes `comments_open` to the template, which then (a) suppresses the per-comment "Customer visible" / "Staff only" badges, since they would misstate what the customer can actually see, and (b) hides the "Share with customer" tick behind a warning banner reading *"Comments are currently visible to everyone… Do not post internal-only notes here."* The checkbox value is still posted and stored, so flipping the config back restores both the filtering and the badges immediately.
- **Read filtering:** `GET issues/view` passes `filter_by(is_customer_visible=True)` to customers; staff receive all comments. **Gated by `COMMENTS_VISIBLE_TO_ALL` (temporary, Aug 2026)** — while that config is true the filter is skipped entirely and customers see every comment. The route passes `comments_open` to the template, which then (a) suppresses the per-comment "Customer visible" / "Staff only" badges, since they would misstate what the customer can actually see, and (b) hides the "Share with customer" tick behind a warning banner reading *"Comments are currently visible to everyone… Do not post internal-only notes here."*
**Both are gated on `viewer_is_our_staff` (Aug 2026), not on `role != 'customer'`.** `can_edit` is true for a **Customer Inspector assigned to the issue**, so that account was being shown the staff comment form complete with an internal-process warning and the internal visibility badges. Neither means anything to a customer — nothing they write was ever private — and both expose how we work. The comment box itself is unaffected; only the internal chrome is hidden.
`viewer_is_our_staff` is one `{% set %}` at the top of the template, written as an explicit **allowlist**`current_user.role in ['admin','director','project_manager','auditor','inspector']` — for two reasons:
- **It fails closed.** The obvious form, `not current_user.is_customer_account`, fails **open**: if that attribute is missing for any reason — most realistically a process still running an older `models/user.py` while templates have already reloaded — Jinja yields `Undefined`, `not Undefined` is true, and the internal text renders for exactly the accounts it must be hidden from. This was observed in practice. An allowlist of literal role strings can only be true for a role actually listed.
- **`external_inspector` is absent on purpose, and this is NOT a rule-87 violation.** Rule 87 governs capability and scoping checks, where a Customer Inspector must behave exactly like our own inspector. This asks a different question — *does this person work for us?* — which is the one place the two roles genuinely differ. Do not add `external_inspector` to this list. The checkbox value is still posted and stored, so flipping the config back restores both the filtering and the badges immediately.
### List filter preservation (Aug 2026)
+29 -3
View File
@@ -23,6 +23,26 @@
preservation). Threaded into every action so an update or delete
returns to the same filtered page, and used by the Back button. #}
{% set back_url = request.args.get('next') or url_for('issues.index') %}
{# Is the viewer OUR staff? Drives the internal-only chrome on this page: the
"comments are visible to everyone" warning and the per-comment
"Customer visible" / "Staff only" badges. Both are instructions about how WE
work and must never reach a customer account.
Written as an explicit ALLOWLIST of our own roles, deliberately:
* It FAILS CLOSED. The obvious form, `not current_user.is_customer_account`,
fails OPEN — if the attribute is missing for any reason (a process still
running an older models/user.py after a template-only reload, say) Jinja
yields Undefined, `not Undefined` is true, and the internal text is shown
to exactly the people it must be hidden from. An allowlist of literal role
strings can only ever be true for a role we listed.
* `external_inspector` is absent ON PURPOSE. This is NOT the rule-87 case:
rule 87 is about capability/scoping, where a Customer Inspector must
behave exactly like our own inspector. Here the question is "does this
person work for us?", which is the one place the two genuinely differ.
Do not "fix" this by adding external_inspector to the list. #}
{% set viewer_is_our_staff = current_user.role in
['admin', 'director', 'project_manager', 'auditor', 'inspector'] %}
<div class="row">
{# ══════════════════════════════════ LEFT COLUMN ══════════════════════════════════ #}
@@ -244,7 +264,7 @@
While comments_open is set, EVERY comment is visible to the
customer, so the old "Staff only" badge would be a lie. It is
suppressed rather than shown incorrectly. #}
{% if current_user.role != 'customer' and not comments_open %}
{% if viewer_is_our_staff and not comments_open %}
{% if c.is_customer_visible %}
<span class="badge bg-success bg-opacity-10 text-success border border-success"
style="font-size:.6rem;" title="Customer can see this comment">
@@ -299,8 +319,14 @@
the "Share with customer" tick decides nothing. Saying so plainly
matters: a staff member must not write something they believe is
private. The checkbox is still posted and recorded, so turning the
setting off restores its meaning immediately. #}
{% if comments_open %}
setting off restores its meaning immediately.
OUR STAFF ONLY. `can_edit` is also true for a Customer Inspector
assigned to the issue, and this banner is an internal-process
warning ("do not post internal-only notes") — showing it to a
customer account exposes how we work and reads as nonsense to
them, since nothing they write was ever private. #}
{% if comments_open and viewer_is_our_staff %}
<div class="alert alert-warning py-2 px-3 small mb-2">
<i class="bi bi-eye me-1"></i>
<strong>Comments are currently visible to everyone,</strong> including