Aug 18 - Fix customer roles's notification radio buttons issue
This commit is contained in:
@@ -440,7 +440,9 @@ Inherit is the default and the safe state, so the table shipped empty and change
|
||||
|
||||
Helpers in `app/models/user_notification_matrix.py`: `overrides_for_user(user_id)` → `{event: bool}` (the editor), `overrides_for_event(event_type)` → `{user_id: bool}` (one query per dispatch, fails soft to `{}`), `set_overrides(user_id, {event: True|False|None})` (does **not** commit — caller owns the transaction, same contract as `notify()`).
|
||||
|
||||
Edited admin-side on the account's Customer Management page as a tri-state (Inherit / On / Off) with the global column's current value shown under "Inherit". Staff roles are unaffected — they use the global matrix alone; `NotificationPreference` remains a different question (how to deliver, not whether to route).
|
||||
Edited admin-side on the account's Customer Management page as a tri-state (Inherit / On / Off) with the global column's current value shown under "Inherit", plus Set-every-row shortcuts. **Each option is a `<label>` filling its table cell** — a bare centred `<input type=radio>` was effectively unclickable at touch/narrow widths, which is what made the editor look broken (see the `ipad_responsive.css` note below).
|
||||
|
||||
**Enforced in `notify()`, not only in `notify_by_matrix()` (Aug 2026).** A customer account's matrix governs every path that reaches it: matrix broadcasts, follower fan-out (`_notify_followers`), and direct assignee notifications all end at `notify()`. Gating only the matrix meant the editor offered rows — "Issue follow update", "Issue assigned" — that read as Off while the notifications kept arriving. Only an explicit `False` suppresses; no row means inherit. The recipient test uses `getattr(recipient, 'is_customer_account', False)` so an unavailable attribute **sends** rather than silently dropping. Staff roles are unaffected — they use the global matrix alone; `NotificationPreference` remains a different question (how to deliver, not whether to route).
|
||||
|
||||
### AuditLog
|
||||
|
||||
@@ -1679,6 +1681,7 @@ timeout = 30
|
||||
| 85 | **`next_due_date` is mutable state, `end_date` is a fixed boundary — never conflate them** | `fulfill()` rewrites `next_due_date` after every completed inspection; `end_date` is set by the manager and never touched by the app. The old single label "Start / Due Date" said both at once, which is what users reported as confusing. The label now follows context — `form.next_due_date.label.text` is set to "Start Date" in `create()` and "Next Due Date" in `edit()`. Do not rename the `next_due_date` column to match a label: it is indexed, it is the API payload key the iPad decodes, and the reminder cron filters on it. |
|
||||
| 87 | **Never write `role == 'inspector'` — use `user.is_inspector` (`User.INSPECTOR_ROLES`)** | phase49 added `external_inspector`, which must behave as an inspector everywhere. An equality check silently drops it into the *privileged* branch of every `if inspector: scope … else: org-wide` block — i.e. a third-party inspector would see **every contract in the system**. This is a fail-OPEN mistake: nothing errors, the data just leaks. The sweep converted ~44 Python sites and 7 template sites; the only surviving `== 'inspector'` literals are the matrix docstring, the `MATRIX_DEFAULTS` mirror comprehension, and the default-checked box in `admin/broadcast.html`. Query-level checks use `User.role.in_(User.INSPECTOR_ROLES)` (never `filter_by(role='inspector')`). A **new** `app/api/*` blueprint's `_ALLOWED_ROLES` must include `external_inspector`, same as rule 79 requires for `auditor`. |
|
||||
| 88 | **`app/enrollment/` writes no DB row and has exactly ONE read — keep the vertical slice sealed** | The enrollment form describes accounts that do NOT exist yet (no contract, facility or user to key a row against), so it stores flat JSON in `ENROLLMENT_DIR` and owns its own templates. The single permitted model access is `mailer._admin_recipients()` reading active `admin` users to address the new-enrollment alert — function-local, read-only, and guarded so a DB failure cannot break a submission. Adding a model/migration for enrollment, or letting the public POST **create** Users, would couple an unauthenticated endpoint to the account system — the exact thing the separation buys. If enrollment must ever provision accounts, do it as a separate admin-triggered action that reads a stored submission. Submission ids are filesystem paths: validate against `_ID_RE` before every open (path traversal). See §24. |
|
||||
| 98 | **Never set `display` on a native checkbox or radio to give it a touch target** | `ipad_responsive.css` had `input[type=radio] { min-height: 44px; display: inline-flex }` inside a `(pointer: coarse), (max-width: 1194px)` query. Replacing a radio's intrinsic box with a flex container leaves the glyph painting at ~16px while the element claims 44px, so the visible dot and the hit area stop coinciding and taps land on nothing — the per-account notification matrix looked entirely unclickable because of it. Grow the target with `transform: scale()` + margin, or wrap the input in a `<label>` that fills the cell. |
|
||||
| 97 | **The template list's Edit modal (`/rename`) is a THIRD edit path — keep it in sync with create and the form editor** | The modal on the template list posts to `rename_template`, not `edit_template`, so a field added only to the two WTForms pages is invisible to the people who edit templates from the list. It carries a hidden `contracts_present=1` marker: an empty selection with the marker means "make this shared", while a POST without it leaves restrictions untouched — otherwise any other caller of that route would silently share a restricted form with every customer. |
|
||||
| 95 | **A template with NO `template_contracts` rows is SHARED, not hidden** | The empty set means "available on every contract" — that is what makes phase52 additive and why it needed no backfill. Reading it the other way would hide every pre-phase52 form from every contract at once. The convention lives in exactly one place, `InspectionTemplate.available_query()`; every picker, the POST validation behind it, and the mobile API call it rather than writing their own filter. A facility with no contract gets shared forms only (fail-closed). |
|
||||
| 96 | **An explicit `?project_id=` / `?facility_id=` filter must still be intersected with the caller's own scope** | Accepting a caller-supplied contract filter *instead of* their scope is a leak, not a filter: a Customer Inspector could pass another customer's facility id and get that customer's form names back. `_visible_templates()` returns `[]` for an out-of-scope contract — empty rather than an error, so the endpoint does not confirm the contract exists either. Applies to any future endpoint that takes a scope-shaped query parameter. |
|
||||
|
||||
@@ -66,6 +66,28 @@ def overrides_for_user(user_id) -> dict:
|
||||
}
|
||||
|
||||
|
||||
def override_for(user_id, event_type):
|
||||
"""One account's answer for one event: True, False, or None (inherit).
|
||||
|
||||
Used by notify() to enforce the override on EVERY delivery path, not just
|
||||
matrix broadcasts. Best-effort: any failure returns None (inherit), so a
|
||||
lookup problem can never silently swallow a notification.
|
||||
"""
|
||||
import logging
|
||||
if not user_id or not event_type:
|
||||
return None
|
||||
try:
|
||||
row = UserNotificationMatrix.query.filter_by(
|
||||
user_id=user_id, event_type=event_type).first()
|
||||
return row.enabled if row is not None else None
|
||||
except Exception as exc:
|
||||
logging.getLogger(__name__).error(
|
||||
'USER MATRIX | single override lookup failed | user=%s event=%s | %s',
|
||||
user_id, event_type, exc,
|
||||
)
|
||||
return None
|
||||
|
||||
|
||||
def overrides_for_event(event_type) -> dict:
|
||||
"""Return {user_id: bool} — every account's override for one event.
|
||||
|
||||
|
||||
@@ -13,15 +13,31 @@
|
||||
/* Minimum 44px touch targets on interactive elements */
|
||||
.btn,
|
||||
.nav-link,
|
||||
.dropdown-item,
|
||||
input[type="checkbox"],
|
||||
input[type="radio"],
|
||||
.form-check-input {
|
||||
.dropdown-item {
|
||||
min-height: 44px;
|
||||
display: inline-flex;
|
||||
align-items: center;
|
||||
}
|
||||
|
||||
/* Checkboxes and radios are deliberately NOT in the rule above.
|
||||
`display: inline-flex` on a native <input type="radio"> replaces its
|
||||
intrinsic box with a flex container: the control keeps painting its glyph
|
||||
at its natural ~16px size while the element claims a 44px-tall box, so the
|
||||
visible dot and the actual hit area stop coinciding. Taps land next to the
|
||||
control and nothing happens — which is exactly how the per-account
|
||||
notification matrix (Inherit / On / Off) came to look unclickable.
|
||||
|
||||
Grow the target without touching `display`: keep the native box, scale the
|
||||
glyph up, and give it margin so neighbouring options stay separable. Any
|
||||
control that needs a genuinely large tap area should wrap the input in a
|
||||
<label> that fills the cell (see customers/manage.html). */
|
||||
input[type="checkbox"],
|
||||
input[type="radio"],
|
||||
.form-check-input {
|
||||
transform: scale(1.35);
|
||||
margin: 6px;
|
||||
}
|
||||
|
||||
/* Slightly larger form controls for finger input */
|
||||
.form-control,
|
||||
.form-select {
|
||||
|
||||
@@ -318,6 +318,34 @@
|
||||
action="{{ url_for('customers.save_notifications', customer_id=customer.id) }}">
|
||||
<input type="hidden" name="csrf_token" value="{{ csrf_token() }}">
|
||||
|
||||
<style>
|
||||
/* The whole cell is the control. Padding (not min-height on the
|
||||
input) gives the touch target, so the native radio keeps its
|
||||
own box — see the note in ipad_responsive.css. */
|
||||
.matrix-opt {
|
||||
display: block;
|
||||
padding: .55rem .25rem;
|
||||
margin: 0;
|
||||
cursor: pointer;
|
||||
text-align: center;
|
||||
}
|
||||
.matrix-opt:hover { background: rgba(13,110,253,.06); }
|
||||
.matrix-opt input { cursor: pointer; }
|
||||
.matrix-opt-sub {
|
||||
display: block;
|
||||
font-size: .62rem;
|
||||
color: #6c757d;
|
||||
margin-top: 2px;
|
||||
}
|
||||
</style>
|
||||
|
||||
<div class="d-flex flex-wrap align-items-center gap-2 mb-2">
|
||||
<span class="small text-muted">Set every row:</span>
|
||||
<button type="button" class="btn btn-sm btn-outline-secondary" data-matrix-all="inherit">Inherit</button>
|
||||
<button type="button" class="btn btn-sm btn-outline-success" data-matrix-all="on">On</button>
|
||||
<button type="button" class="btn btn-sm btn-outline-danger" data-matrix-all="off">Off</button>
|
||||
</div>
|
||||
|
||||
<div class="table-responsive">
|
||||
<table class="table table-sm align-middle mb-3">
|
||||
<thead class="table-light">
|
||||
@@ -337,23 +365,33 @@
|
||||
<span class="badge bg-warning text-dark ms-1" style="font-size:.6rem;">custom</span>
|
||||
{% endif %}
|
||||
</td>
|
||||
<td class="text-center">
|
||||
<input class="form-check-input" type="radio"
|
||||
name="event_{{ row.event }}" value="inherit"
|
||||
{% if row.override is none %}checked{% endif %}>
|
||||
<div class="text-muted" style="font-size:.62rem;">
|
||||
{{ 'on' if row.global else 'off' }}
|
||||
</div>
|
||||
{# Each option is a <label> filling its whole cell, so the
|
||||
click target is the cell rather than the ~16px glyph. A
|
||||
bare <input> in a centred <td> was effectively unclickable
|
||||
at touch/narrow widths. #}
|
||||
<td class="p-0">
|
||||
<label class="matrix-opt" title="Follow the global matrix">
|
||||
<input class="form-check-input" type="radio"
|
||||
name="event_{{ row.event }}" value="inherit"
|
||||
{% if row.override is none %}checked{% endif %}>
|
||||
<span class="matrix-opt-sub">
|
||||
currently {{ 'on' if row.global else 'off' }}
|
||||
</span>
|
||||
</label>
|
||||
</td>
|
||||
<td class="text-center">
|
||||
<input class="form-check-input" type="radio"
|
||||
name="event_{{ row.event }}" value="on"
|
||||
{% if row.override is true %}checked{% endif %}>
|
||||
<td class="p-0">
|
||||
<label class="matrix-opt" title="Always send this to this account">
|
||||
<input class="form-check-input" type="radio"
|
||||
name="event_{{ row.event }}" value="on"
|
||||
{% if row.override is true %}checked{% endif %}>
|
||||
</label>
|
||||
</td>
|
||||
<td class="text-center">
|
||||
<input class="form-check-input" type="radio"
|
||||
name="event_{{ row.event }}" value="off"
|
||||
{% if row.override is false %}checked{% endif %}>
|
||||
<td class="p-0">
|
||||
<label class="matrix-opt" title="Never send this to this account">
|
||||
<input class="form-check-input" type="radio"
|
||||
name="event_{{ row.event }}" value="off"
|
||||
{% if row.override is false %}checked{% endif %}>
|
||||
</label>
|
||||
</td>
|
||||
</tr>
|
||||
{% endfor %}
|
||||
@@ -396,6 +434,16 @@
|
||||
if (deselectAll) deselectAll.addEventListener('click', function () { setAll(false); });
|
||||
checks.forEach(function (c) { c.addEventListener('change', refreshCount); });
|
||||
|
||||
// ── Notification matrix: set every row at once ──
|
||||
document.querySelectorAll('[data-matrix-all]').forEach(function (btn) {
|
||||
btn.addEventListener('click', function () {
|
||||
var want = btn.getAttribute('data-matrix-all');
|
||||
document.querySelectorAll('.matrix-opt input[type=radio]').forEach(function (r) {
|
||||
if (r.value === want) { r.checked = true; }
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// ── Customer Director: contract → facility cascade ──
|
||||
// Both selects are absent on the inspector view, so bail out rather than
|
||||
// throwing on addEventListener of null (which would kill the handlers above).
|
||||
|
||||
@@ -209,6 +209,28 @@ def notify(
|
||||
the scheduled-inspection "Confirm receipt" email link. In-app
|
||||
notifications are unaffected — this only shapes the email.
|
||||
"""
|
||||
# ── Per-account override (phase51) ───────────────────────────────────
|
||||
# A customer-side account's own notification matrix governs EVERY path
|
||||
# that reaches it, not just matrix broadcasts: follower fan-out
|
||||
# (_notify_followers) and direct assignee notifications both call notify()
|
||||
# straight, so without this the editor would offer rows — "Issue follow
|
||||
# update", "Issue assigned" — that appeared to be off while the
|
||||
# notifications kept arriving.
|
||||
#
|
||||
# Only an explicit `False` suppresses. No row means inherit, which is the
|
||||
# default for every account and leaves behaviour exactly as before. The
|
||||
# getattr fallback is deliberate: if the attribute is unavailable for any
|
||||
# reason we send, never silently drop.
|
||||
if event_type and getattr(recipient, 'is_customer_account', False):
|
||||
from app.models.user_notification_matrix import override_for
|
||||
if override_for(recipient.id, event_type) is False:
|
||||
logger.info(
|
||||
'NOTIFICATION SUPPRESSED | user=%s | event=%s | '
|
||||
'reason=per_account_override_off',
|
||||
recipient.username, event_type,
|
||||
)
|
||||
return
|
||||
|
||||
# Determine digest flag before creating the record.
|
||||
# Digest mode is only respected when individual preferences are in effect.
|
||||
hold_for_digest = (
|
||||
|
||||
Reference in New Issue
Block a user