diff --git a/CLAUDE.md b/CLAUDE.md index ef944a9..1cfd042 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -571,9 +571,9 @@ Management (`/scheduled-inspections/new|edit|delete`) is `@project_manager_requi | `public` | `/f` | **No login.** `GET /` occupant facility summary + `POST //report` occupant issue report; `GET /area/` per-area summary + `POST /area//report` (Phase 39, files with `area_id` set). Report form accepts **up to 5 photos** (`_save_report_photos()` → `photo_path` + `mobile_photo_paths`). All report POSTs rate-limited `5/hour`, honeypot-guarded. Resolves ACTIVE facility (area's parent must be active) by `public_token` or 404. | | `projects` | `/projects` | CRUD + customer assignment management + notification-recipient add/remove (`//notify-recipients/add`, `/notify-recipients//remove` — admin only) | | `customers` | `/customers` | **Owns BOTH customer roles (Phase 51).** `GET /` list (both roles, role badge + per-role scope column), `GET/POST /new` invite (role select: Customer Director / Customer Inspector — same invitation flow for both), `/set-password/`, `GET /` manage, `//edit`, `POST //assignments/add` + `/assignments//remove` (**director only** — `CustomerAssignment`), `POST //contracts` (**inspector only** — replaces the whole `InspectorAssignment` set, rule 59 semantics), `POST //notifications` (per-account matrix overrides), `POST //switch-role` (**admin only** — mirrors contracts across, revokes tokens/devices), `POST //toggle-active`, `POST //resend-invite`, import CSV | -| `inspections` | `/inspections` | list, start, execute, view, PDF export, flag-issue, save-draft (AJAX), flag-followup, reinspect, upload-photo (AJAX) | +| `inspections` | `/inspections` | list, start, execute, view, PDF export, flag-issue, save-draft (AJAX), flag-followup, reinspect, upload-photo (AJAX), **`POST /bulk`** (bulk export-PDF / request-follow-up / clear-follow-up / delete from the list) | | `templates` | `/templates` | list, create, edit, delete, form editor, preview | -| `issues` | `/issues` | list, view, create, update, verify, comment, follow/unfollow, verification queue, bulk-verify, delete, quick-assign. **verify / bulk-verify / verification-queue are `@issue_manager_required` (admin/director/auditor); delete stays `@supervisor_required` (admin/director).** | +| `issues` | `/issues` | list, view, create, update, verify, comment, follow/unfollow, verification queue, bulk-verify, delete, quick-assign, **`POST /bulk`** (bulk assign / status / verify / delete from the list). **verify / bulk-verify / verification-queue are `@issue_manager_required` (admin/director/auditor); delete stays `@supervisor_required` (admin/director).** | | `notifications` | `/notifications` | list, mark-read, preferences, send-digest (cron), check-sla (cron), cleanup-tokens (cron) | | `audit` | `/audit` | list (admin only), view, purge | | `reports` | `/reports` | index, facility report, scorecard, CSV/PDF/Excel export, issues-aging, sla-compliance, followup-closure, facility summary PDF | @@ -612,6 +612,10 @@ Management (`/scheduled-inspections/new|edit|delete`) is `@project_manager_requi So with no DNS work an invite from `jqc.govservicesinc.com` sends `From: "Gov Services QC" ` (branded name, deliverable address). After that domain's SPF `include:` + DKIM are live, add it to `SENDER_AUTHORIZED_DOMAINS` and it upgrades to `` — no code change. Falls back to the bare authenticated sender string for unparseable hosts (localhost, empty). Edit `BRAND_NAMES` / `SENDER_AUTHORIZED_DOMAINS` as brands and DNS come online. See rule 64. +### `decorators.py` — `return_url(fallback)` + +Reads the `next` value a list-page action carried (POST body first, then query string), validates it with `safe_redirect_url`, and falls back. This is what makes an edit or delete return to the **filtered** list instead of the bare index. `next` is the FULL list URL — never a reconstructed argument set — so adding a filter to either list page needs no change here. See §18 "List filter preservation". + ### `scope.py` `get_customer_scope(user)` — returns `list[int]` facility IDs for customers, `None` for non-customers. `get_inspector_scope(user)` — returns `list[int]` facility IDs for inspectors (empty list = no assignments = no access), `None` for non-inspectors. Derived from `InspectorAssignment` rows → project → active facilities. @@ -1335,6 +1339,40 @@ Rendered in `dashboard.html` for `current_user.role == 'customer'`. Uses a Boots - **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. +### List filter preservation (Aug 2026) + +Filtering a list, then editing or deleting a row, used to dump the user back on the **unfiltered** index. Every list-page action now round-trips the list URL. + +**The mechanism, end to end:** +1. `current_url()` — Jinja global registered in `app/__init__.py`, returns `request.full_path` with a bare trailing `?` stripped. The list templates put it in a `` on every action form, and append `?next=` to every link into a detail page. +2. The detail templates (`issues/view.html`, `inspections/view.html`) set `{% set back_url = request.args.get('next') or url_for('.index') %}` once, and thread it into their own action forms **and** the Back button. +3. `return_url(fallback)` (§8) resolves it after the action; `_view_url(id)` in each blueprint re-attaches `next` when an action redirects back to the detail page, so the chain survives an update. + +**Carry the whole URL, not the filters.** The old unfollow form rebuilt `next` with an 11-argument `url_for(...)` that had to be hand-edited whenever a filter was added — and silently dropped any filter nobody remembered. `current_url()` cannot drift. + +`safe_redirect_url` still guards every hop, so a crafted `next=https://evil.com` falls back to the index (rule 15) — verified. + +The inspections list also keeps its older `sessionStorage['insp_list_back_url']` fallback for links created before `next` existed, but a server-provided `next` **wins**: `view.html` emits `var hasNext = true;` and skips the sessionStorage read, otherwise a stale stored URL would override the list the page was actually opened from. + +### Bulk actions on the list pages (Aug 2026) + +Both list pages carry a bulk toolbar above the table with per-row checkboxes. + +| Page | Actions | Permission | +|---|---|---| +| Issues (`POST /issues/bulk`) | assign, set status, verify & close, delete | assign/status/verify: admin/director/auditor · delete: admin/director | +| Inspections (`POST /inspections/bulk`) | export selected to PDF, request follow-up (shared note), clear follow-up, delete | export: anyone who can see the list · rest: admin/director | + +**The toolbar form sits OUTSIDE the table — this is load-bearing (rule 91).** Row checkboxes join it with the HTML5 `form="issuesBulkForm"` attribute rather than being wrapped by it. Wrapping the table would nest the per-row delete/unfollow forms inside the bulk form, and browsers silently discard nested forms (rule 9) — the row actions would stop working with no error anywhere. + +**Shared partials, not four copies.** Both lists have classic *and* modern variants, so the markup lives in `templates/partials/bulk_issues_toolbar.html`, `bulk_inspections_toolbar.html` and `bulk_select_js.html`; each of the four list templates includes them. The JS is generic (`.bulk-check`, `.bulk-check-all`, `.bulk-count`, `[data-bulk-action]`, `[data-bulk-confirm]`) and supports shift-click range selection; it disables the action buttons while nothing is selected, so an empty POST can't cost a page round trip. + +**Partial-failure policy: act, skip, report exact counts** — never block the batch on one ineligible row, never silently drop rows. `_flash_bulk()` in each blueprint emits the one message shape ("3 issues verified and closed. 2 skipped (not awaiting verification)."). Rows are skipped when the action does not apply (already in that status, not submitted yet, already flagged); *permission* is checked per action, up front, not per row. + +**The inspections bulk route re-applies facility scope to the submitted ids.** The list only ever shows in-scope rows, but the id list arrives in the POST body and is not trusted — without the re-check a crafted request could name any inspection in the system. Issue bulk actions are all manager-level (org-wide access), so they have no per-row scope question. + +**Deletes remove DB rows first, files second** (both blueprints). An orphaned file is recoverable; a file deleted out from under a surviving row is not. `_collect_inspection_photos()` was factored out of the single-delete path so bulk and single delete cannot drift — a miss there leaks storage silently, forever. + ### Inspection List Filters `inspections.index()` accepts five additional query params: `date_from`, `date_to` (ISO date strings), `score_min`, `score_max` (0–100 floats), `inspector_id` (int). Inspector filter is suppressed when the viewer has the `inspector` role (they always see their own only). The `inspectors` variable is passed to the template only for non-inspector roles so the dropdown is conditionally rendered. @@ -1577,6 +1615,8 @@ 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. | +| 91 | **A bulk-action form must live OUTSIDE the table; row checkboxes join it via the HTML5 `form=` attribute** | Wrapping the table in the bulk form nests the per-row delete/unfollow forms inside it, and browsers **silently discard** nested forms (rule 9) — the row buttons would post nothing, with no console error and no server log. `
` sits above the table and each checkbox carries `form="issuesBulkForm"`. Same for `inspectionsBulkForm`. Applies to all four list templates (classic + modern). | +| 92 | **Bulk deletes: DB rows first, storage files second** | Collect the keys, `db.session.delete()` every row, `commit()`, and only then `storage.delete()`. Deleting files first means a failed/rolled-back commit leaves surviving rows pointing at missing photos. `_collect_inspection_photos()` is shared by the single and bulk inspection delete paths precisely so the two cannot drift — a key missed there is an invisible permanent storage leak. | | 89 | **`User.CUSTOMER_ROLES` is for ACCOUNT MANAGEMENT; `role == 'customer'` is for CAPABILITY — never swap them** | The inverse of rule 87, and it fails in both directions. Widening a capability check to `CUSTOMER_ROLES` hands a third-party Customer Inspector the customer portal (fail-OPEN, nothing errors). Narrowing an account-management check back to `'customer'` strands every Customer Inspector in a page that no longer lists or edits them (fail-closed, but invisible until someone looks for a missing account). `CUSTOMER_ROLES` / `is_customer_account` appear ONLY in: the `/customers` list query, its route guards, and the `auth.list_users` exclusion. Everything else — portal gates, `@customer_required`, `get_customer_scope()`, support chat, `notify_customers_for_facility()`, the customer branch of every `app/api/*` scope check — keeps the equality test, because a Customer Inspector is an **inspector** there (rule 87 already routes it correctly). | | 90 | **A per-account notification opt-IN must survive a globally-OFF column** | `notify_by_matrix()` skips a role column early when the matrix says off. For the two customer columns that early `continue` has to also ask whether anyone opted in (`any(overrides.values())`), or the override saves, displays as on, and never sends — a silent failure with no error anywhere. Equally, `notify_customers_for_facility()` re-queries recipients from assignment rows, so `notify_by_matrix()` must hand it `allowed_user_ids` or the facility-scoped path bypasses every override. Both halves are needed; either one alone leaves a hole. See §11. | | 81 | **Photo timestamp/geo overlay is burned at UPLOAD, never on `PATCH /issues//photos`** | That PATCH receives only path strings — the bytes are already in storage and the payload carries no capture metadata. Burning there would need a read-modify-write per key plus an overwrite-in-place primitive (`storage.save()` mints a NEW uuid key, and §22 requires key == DB path), and would risk a **double burn** since the endpoint is deliberately idempotent/retry-safe (rule 45). Stamp in `POST /photos/upload`, where the raw bytes + EXIF are in hand and each call writes exactly one already-stamped object. Stamping failures must always fall back to storing the ORIGINAL bytes — never lose a photo to a stamping bug. See §23. | diff --git a/app/__init__.py b/app/__init__.py index 020f975..aa14cc2 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -143,6 +143,15 @@ def create_app(config_name='default'): from app.utils import storage as _storage app.jinja_env.globals['media_url'] = _storage.media_url + # Current page URL including its query string — what list pages hand to + # their actions as `next` so filters survive an edit/delete round trip + # (see utils/decorators.return_url). full_path always appends '?', which + # is harmless but makes for ugly links, so strip a bare trailing one. + def _current_url(): + from flask import request + return request.full_path.rstrip('?') if request else '' + app.jinja_env.globals['current_url'] = _current_url + # ── Design A/B test wiring (phase48) ────────────────────────────────── # Index the modern/ override templates once at boot, so get_template() # never has to touch the filesystem per request. diff --git a/app/routes/inspections.py b/app/routes/inspections.py index c68705e..96244d5 100644 --- a/app/routes/inspections.py +++ b/app/routes/inspections.py @@ -15,7 +15,7 @@ from app.models.project import Project from app.models.issue import Issue from app.models.user import User from app.utils.forms import StartInspectionForm, IssueForm -from app.utils.decorators import supervisor_required +from app.utils.decorators import supervisor_required, return_url from app.utils.pdf_export import generate_inspection_pdf, generate_inspections_list_pdf from app.utils.notifications import notify, notify_customers_for_facility, notify_by_matrix from app.models.notification import ( @@ -477,7 +477,7 @@ def execute(inspection_id): return redirect(url_for('inspections.index')) if inspection.status == 'completed': - return redirect(url_for('inspections.view', inspection_id=inspection_id)) + return redirect(_view_url(inspection_id)) template = inspection.template form_fields = template.get_form_schema() @@ -600,7 +600,7 @@ def execute(inspection_id): f'status=completed; score={score}') flash('Inspection submitted successfully!', 'success') - return redirect(url_for('inspections.view', inspection_id=inspection_id)) + return redirect(_view_url(inspection_id)) else: _save_draft(inspection, responses) @@ -1220,6 +1220,231 @@ def export_pdf(inspection_id): # ── Flag / clear follow-up required ────────────────────────────────────────── +def _view_url(inspection_id): + """inspections.view URL that carries the list `next` through. + + Actions posted from the detail page redirect back to that same page; + re-attaching `next` is what keeps its Back button (and the next action) + pointed at the filtered list the user arrived from. + """ + nxt = request.form.get('next') or request.args.get('next') + if nxt: + return url_for('inspections.view', inspection_id=inspection_id, next=nxt) + return url_for('inspections.view', inspection_id=inspection_id) + + +def _collect_inspection_photos(inspection): + """Relative storage keys owned by an inspection, for cleanup after delete. + + Two sources: image field values inside the submitted form data (stored as + `uploads/...` strings in the notes JSON), and the primary photo of each + issue flagged during the inspection. Shared by the single and bulk delete + paths so they cannot drift — a miss here leaves orphaned files in storage + forever, and it is invisible. + """ + paths = [] + if inspection.notes: + try: + notes_data = json.loads(inspection.notes) + form_data = notes_data.get('_form_data', {}) if isinstance(notes_data, dict) else {} + for val in form_data.values(): + if isinstance(val, str) and val.startswith('uploads/'): + paths.append(val) + except (json.JSONDecodeError, TypeError): + pass + + for issue in inspection.issues.all(): + if issue.photo_path: + paths.append(issue.photo_path) + return paths + + +# ── Bulk actions from the inspections list ─────────────────────────────────── + +@bp.route('/bulk', methods=['POST']) +@login_required +def bulk_action(): + """Apply one action to every ticked inspection on the list page. + + Partial-failure policy: act on every eligible row, skip the rest, and + report exact counts. Permission is checked per ACTION (all are + admin/director level except the PDF export, which anyone who can see the + list may run); `skipped` therefore means "this row was not in a state the + action applies to". + """ + back = return_url(url_for('inspections.index')) + action = request.form.get('action', '') + ids = request.form.getlist('inspection_ids', type=int) + + if not ids: + flash('No inspections selected.', 'warning') + return redirect(back) + + supervisor = current_user.role in ('admin', 'director') + allowed = { + 'export': True, # read-only, already scoped below + 'delete': supervisor, + 'flag_followup': supervisor, + 'clear_followup': supervisor, + } + if action not in allowed: + flash('Unknown bulk action.', 'danger') + return redirect(back) + if not allowed[action]: + flash('You do not have permission for that bulk action.', 'danger') + return redirect(back) + + q = Inspection.query.options( + joinedload(Inspection.facility), + joinedload(Inspection.template), + joinedload(Inspection.inspector), + ).filter(Inspection.id.in_(ids)) + + # Re-apply the viewer's facility scope to the SELECTED ids. The list page + # only ever shows in-scope rows, but the id list arrives in the POST body + # and must not be trusted — a crafted request could otherwise name any + # inspection in the system. + if current_user.is_inspector: + fids = get_inspector_scope(current_user) or [] + q = q.filter(Inspection.facility_id.in_(fids)) if fids else q.filter(False) + elif current_user.role == 'customer': + fids = get_customer_scope(current_user) or [] + q = q.filter(Inspection.facility_id.in_(fids)) if fids else q.filter(False) + + inspections = q.order_by(Inspection.inspection_date.desc()).all() + out_of_scope = len(ids) - len(inspections) + changed = 0 + skipped = out_of_scope + + # ── Export selected to PDF ─────────────────────────────────────────── + if action == 'export': + if not inspections: + flash('None of the selected inspections are available to you.', 'warning') + return redirect(back) + from flask import Response + pdf = generate_inspections_list_pdf( + inspections, + f'Selected inspections ({len(inspections)})', + ) + log_action(ACTION_EXPORT, 'Inspection', None, 'bulk PDF export', + f'ids={[i.id for i in inspections]}') + return Response( + pdf, + mimetype='application/pdf', + headers={'Content-Disposition': + 'attachment; filename="selected_inspections.pdf"'}, + ) + + # ── Delete ─────────────────────────────────────────────────────────── + if action == 'delete': + from app.utils import storage + photo_paths = [] + for insp in inspections: + photo_paths.extend(_collect_inspection_photos(insp)) + log_action(ACTION_DELETE, 'Inspection', insp.id, + f'{insp.template.name if insp.template else "—"} @ ' + f'{insp.facility.name if insp.facility else "—"}', + f'bulk deleted by {current_user.username}') + db.session.delete(insp) + changed += 1 + db.session.commit() + # Files only after the rows are gone — an orphaned file is recoverable, + # a deleted file belonging to a surviving row is not. + for rel_path in photo_paths: + storage.delete(rel_path) + _flash_bulk(changed, skipped, 'permanently deleted') + + # ── Request follow-up ──────────────────────────────────────────────── + elif action == 'flag_followup': + note = request.form.get('follow_up_note', '').strip() or None + for insp in inspections: + # Same two guards as the single-inspection route: nothing to follow + # up on before submission, and a repeat request must not overwrite + # the pending one's note or attribution. + if insp.status != 'completed' or insp.follow_up_required: + skipped += 1 + continue + insp.follow_up_required = True + insp.follow_up_note = note + insp.follow_up_requested_by = current_user.id + insp.follow_up_requested_at = now_eastern() + changed += 1 + db.session.commit() + + for insp in inspections: + if insp.follow_up_requested_by != current_user.id or not insp.follow_up_required: + continue + body = (f'{current_user.display_name} has requested a follow-up ' + f're-inspection of "{insp.template.name if insp.template else "—"}" ' + f'at {insp.facility.name if insp.facility else "—"}.' + + (f' Note: {note}' if note else '')) + inspector = db.session.get(User, insp.inspector_id) + if inspector and inspector.id != current_user.id: + notify( + recipient = inspector, + title = f'Follow-Up Required: Inspection #{insp.id}', + body = body, + link = url_for('inspections.view', inspection_id=insp.id), + inspection_id = insp.id, + event_type = EVENT_INSPECTION_DONE, + send_email = True, + ) + # Through the matrix, not straight to managers — rule 73, so + # per-contract recipients fire here exactly as they do for a + # single request. + notify_by_matrix( + event_type = EVENT_FOLLOWUP_REQUESTED, + title = f'Follow-Up Requested: Inspection #{insp.id}', + body = body, + link = url_for('inspections.view', inspection_id=insp.id), + inspection_id = insp.id, + facility_id = insp.facility_id, + exclude_user_ids = {current_user.id, + inspector.id if inspector else None} - {None}, + ) + log_action(ACTION_UPDATE, 'Inspection', insp.id, + f'{insp.template.name if insp.template else "—"}', + f'bulk follow_up_required=True by {current_user.username}') + db.session.commit() # notify() does not commit — rule 70 + _flash_bulk(changed, skipped, 'flagged for follow-up', + skip_reason='not submitted, or already flagged') + + # ── Clear follow-up ────────────────────────────────────────────────── + elif action == 'clear_followup': + for insp in inspections: + if not insp.follow_up_required: + skipped += 1 + continue + insp.follow_up_required = False + insp.follow_up_note = None + insp.follow_up_requested_by = None + insp.follow_up_requested_at = None + changed += 1 + log_action(ACTION_UPDATE, 'Inspection', insp.id, + f'{insp.template.name if insp.template else "—"}', + f'bulk follow_up cleared by {current_user.username}') + db.session.commit() + _flash_bulk(changed, skipped, 'cleared of the follow-up flag', + skip_reason='not flagged') + + current_app.logger.info( + 'INSPECTIONS | bulk | action=%s user=%s selected=%s changed=%s skipped=%s', + action, current_user.username, len(ids), changed, skipped, + ) + return redirect(back) + + +def _flash_bulk(changed, skipped, verb, skip_reason='no change needed'): + """One consistent result message for every bulk action.""" + if not changed and not skipped: + flash('Nothing to do.', 'info') + return + parts = [f'{changed} inspection{"s" if changed != 1 else ""} {verb}'] + if skipped: + parts.append(f'{skipped} skipped ({skip_reason})') + flash('. '.join(parts) + '.', 'success' if changed else 'warning') + + @bp.route('//flag-followup', methods=['POST']) @login_required def flag_followup(inspection_id): @@ -1245,12 +1470,12 @@ def flag_followup(inspection_id): # Nothing to follow up on until the inspection has been submitted. if inspection.status != 'completed': flash('You can only request a follow-up on a completed inspection.', 'warning') - return redirect(url_for('inspections.view', inspection_id=inspection_id)) + return redirect(_view_url(inspection_id)) # Don't let a repeat request overwrite the note/attribution of a pending # one — the flag is already raised and staff are already on it. if inspection.follow_up_required: flash('A follow-up has already been requested for this inspection.', 'info') - return redirect(url_for('inspections.view', inspection_id=inspection_id)) + return redirect(_view_url(inspection_id)) elif current_user.role not in ('admin', 'director'): abort(403) @@ -1312,7 +1537,7 @@ def flag_followup(inspection_id): flash('Follow-up re-inspection requested. The team has been notified.', 'success') else: flash('Follow-up inspection required flag set.', 'warning') - return redirect(url_for('inspections.view', inspection_id=inspection_id)) + return redirect(_view_url(inspection_id)) @bp.route('//clear-followup', methods=['POST']) @@ -1332,7 +1557,7 @@ def clear_followup(inspection_id): f'{inspection.template.name} @ {inspection.facility.name}', 'follow_up_required=False (cleared)') flash('Follow-up flag cleared.', 'success') - return redirect(url_for('inspections.view', inspection_id=inspection_id)) + return redirect(_view_url(inspection_id)) # ── Start a re-inspection (linked to parent) ────────────────────────────────── @@ -1378,20 +1603,7 @@ def delete(inspection_id): template_name = inspection.template.name inspector_name = inspection.inspector.username - photo_paths = [] - if inspection.notes: - try: - notes_data = json.loads(inspection.notes) - form_data = notes_data.get('_form_data', {}) if isinstance(notes_data, dict) else {} - for val in form_data.values(): - if isinstance(val, str) and val.startswith('uploads/'): - photo_paths.append(val) - except (json.JSONDecodeError, TypeError): - pass - - for issue in inspection.issues.all(): - if issue.photo_path: - photo_paths.append(issue.photo_path) + photo_paths = _collect_inspection_photos(inspection) db.session.delete(inspection) db.session.commit() @@ -1418,4 +1630,4 @@ def delete(inspection_id): f'has been permanently deleted.', 'success' ) - return redirect(url_for('inspections.index')) \ No newline at end of file + return redirect(return_url(url_for('inspections.index'))) \ No newline at end of file diff --git a/app/routes/issues.py b/app/routes/issues.py index a47c732..374c897 100644 --- a/app/routes/issues.py +++ b/app/routes/issues.py @@ -15,7 +15,8 @@ from app.models.notification import ( EVENT_CUSTOMER_ISSUE_UPDATED, ) from app.utils.forms import IssueForm, IssueUpdateForm -from app.utils.decorators import supervisor_required, issue_manager_required +from app.utils.decorators import (supervisor_required, issue_manager_required, + return_url) from app.utils.notifications import notify, notify_customers_for_facility, notify_by_matrix from app.utils.audit import log_action, ACTION_CREATE, ACTION_UPDATE, ACTION_DELETE, ACTION_EXPORT from app.utils.pdf_export import generate_issues_list_pdf @@ -430,7 +431,7 @@ def view(issue_id): comment_body = request.form.get('update_notes', '').strip() if not comment_body: flash('Comment cannot be empty.', 'warning') - return redirect(url_for('issues.view', issue_id=issue_id)) + return redirect(_view_url(issue_id)) comment = IssueComment( issue_id=issue.id, user_id=current_user.id, @@ -444,7 +445,7 @@ def view(issue_id): f'#{issue.id}', 'customer comment added') flash('Comment posted.', 'success') - return redirect(url_for('issues.view', issue_id=issue_id)) + return redirect(_view_url(issue_id)) form = IssueUpdateForm(obj=issue) staff = User.query.filter(User.role.in_(['director', 'inspector', 'external_inspector', 'auditor'])).order_by(User.username).all() @@ -685,7 +686,7 @@ def view(issue_id): f'#{issue.id} in {issue.area.name if issue.area else issue.resolved_facility.name if issue.resolved_facility else '—'}', f'status={issue.status}; handler={issue.handler_type}; assigned_to={issue.assigned_to}') flash('Issue updated.', 'success') - return redirect(url_for('issues.view', issue_id=issue_id)) + return redirect(_view_url(issue_id)) is_following = issue.is_followed_by(current_user) @@ -728,7 +729,7 @@ def follow(issue_id): flash('You are now following this issue and will receive notifications for any updates.', 'success') else: flash('You are already following this issue.', 'info') - return redirect(url_for('issues.view', issue_id=issue_id)) + return redirect(_view_url(issue_id)) # ── Unfollow ────────────────────────────────────────────────────────────────── @@ -878,7 +879,7 @@ def create(): ) db.session.commit() flash('Issue created.', 'success') - return redirect(url_for('issues.index')) + return redirect(return_url(url_for('issues.index'))) return render_template('issues/form.html', form=form, title='Log New Issue', projects=projects, selected_project_id=selected_project_id) @@ -897,7 +898,7 @@ def verify(issue_id): if issue.status not in ('resolved', 'pending_verification'): flash('Only resolved or pending-verification issues can be verified.', 'warning') - return redirect(url_for('issues.view', issue_id=issue_id)) + return redirect(_view_url(issue_id)) note = request.form.get('verification_note', '').strip() or None @@ -917,7 +918,21 @@ def verify(issue_id): f'#{issue_id} in {issue.area.name if issue.area else issue.resolved_facility.name if issue.resolved_facility else '—'}', f'verified_by={current_user.username}') flash(f'Issue #{issue_id} verified and closed.', 'success') - return redirect(url_for('issues.view', issue_id=issue_id)) + return redirect(_view_url(issue_id)) + + +def _view_url(issue_id): + """issues.view URL that carries the list `next` through. + + An update posted from the detail page redirects back to that same detail + page; without re-attaching `next`, the Back button would lose the filters + the user arrived with and the next action from this page would too. Only + added when there is something to carry, so ordinary links stay clean. + """ + nxt = request.form.get('next') or request.args.get('next') + if nxt: + return url_for('issues.view', issue_id=issue_id, next=nxt) + return url_for('issues.view', issue_id=issue_id) @bp.route('/bulk-verify', methods=['POST']) @@ -952,7 +967,180 @@ def bulk_verify(): f'bulk_verified_by={current_user.username}') flash(f'{verified_count} issue{"s" if verified_count != 1 else ""} verified and closed.', 'success') - return redirect(url_for('issues.verification_queue')) + # Reachable from BOTH the verification queue and the issues list, so honour + # the caller's `next` and fall back to the queue as before. + return redirect(return_url(url_for('issues.verification_queue'))) + + +# ── Bulk actions from the issues list ──────────────────────────────────────── + +#: Statuses a bulk status change may set, and what an issue must already be in +#: for the change to mean anything. Moving an issue to the state it is already +#: in is a no-op, so it counts as skipped rather than changed. +_BULK_STATUSES = ('open', 'in_progress', 'resolved', 'pending_verification') + + +@bp.route('/bulk', methods=['POST']) +@login_required +def bulk_action(): + """Apply one action to every ticked issue on the list page. + + Partial-failure policy (matches bulk_verify): act on every eligible row, + skip the rest, and report exact counts — never silently drop rows, and + never let one ineligible row block the batch. + + Permission is checked per ACTION here rather than per row: all four actions + are manager-level, and the roles that hold them have org-wide issue access, + so there is no per-row scope question to answer. `skipped` therefore only + ever means "this row was not in a state the action applies to". + """ + back = return_url(url_for('issues.index')) + action = request.form.get('action', '') + ids = request.form.getlist('issue_ids', type=int) + + if not ids: + flash('No issues selected.', 'warning') + return redirect(back) + + manager = current_user.role in ('admin', 'director', 'auditor') + deleter = current_user.role in ('admin', 'director') + + allowed = { + 'assign': manager, + 'status': manager, + 'verify': manager, + 'delete': deleter, + } + if action not in allowed: + flash('Unknown bulk action.', 'danger') + return redirect(back) + if not allowed[action]: + flash('You do not have permission for that bulk action.', 'danger') + return redirect(back) + + issues = [i for i in (db.session.get(Issue, i_id) for i_id in ids) if i is not None] + missing = len(ids) - len(issues) + changed = 0 + skipped = missing + + # ── Assign ─────────────────────────────────────────────────────────── + if action == 'assign': + raw = request.form.get('assigned_to', '') + user = None + if raw and raw != '0': + user = db.session.get(User, int(raw)) if raw.isdigit() else None + if user is None: + flash('That user no longer exists.', 'danger') + return redirect(back) + + for issue in issues: + if issue.assigned_to == (user.id if user else None): + skipped += 1 + continue + issue.assigned_to = user.id if user else None + changed += 1 + db.session.commit() + + for issue in issues: + if issue.assigned_to == (user.id if user else None) and user: + notify( + recipient = user, + title = f'Issue #{issue.id} assigned to you', + body = (f'{issue.severity.title()}-severity issue at ' + f'{issue.resolved_facility.name if issue.resolved_facility else "—"}: ' + f'{issue.description[:120]}'), + link = url_for('issues.view', issue_id=issue.id), + issue_id = issue.id, + event_type = EVENT_ISSUE_ASSIGNED, + send_email = True, + ) + db.session.commit() # notify() does not commit — rule 70 + + label = user.display_name if user else 'Unassigned' + log_action(ACTION_UPDATE, 'Issue', None, f'bulk assign → {label}', + f'ids={[i.id for i in issues]}; changed={changed}') + _flash_bulk(changed, skipped, f'assigned to {label}') + + # ── Status ─────────────────────────────────────────────────────────── + elif action == 'status': + new_status = request.form.get('status', '') + if new_status not in _BULK_STATUSES: + flash('Please choose a status to set.', 'warning') + return redirect(back) + + for issue in issues: + if issue.status == new_status: + skipped += 1 + continue + old = issue.status + issue.status = new_status + # Keep resolved_at consistent with the status, the same way the + # single-issue update does — a resolved issue with no resolved_at + # breaks the SLA compliance report and the aging buckets. + if new_status == 'resolved' and not issue.resolved_at: + issue.resolved_at = now_eastern() + elif new_status in ('open', 'in_progress'): + issue.resolved_at = None + changed += 1 + log_action(ACTION_UPDATE, 'Issue', issue.id, f'#{issue.id}', + f'bulk status {old} → {new_status} by {current_user.username}') + db.session.commit() + _flash_bulk(changed, skipped, + f'set to {new_status.replace("_", " ").title()}') + + # ── Verify & close ─────────────────────────────────────────────────── + elif action == 'verify': + for issue in issues: + if issue.status not in ('resolved', 'pending_verification'): + skipped += 1 + continue + issue.status = 'resolved' + issue.verified_by = current_user.id + issue.verified_at = now_eastern() + if not issue.resolved_at: + issue.resolved_at = now_eastern() + changed += 1 + log_action(ACTION_UPDATE, 'Issue', issue.id, f'#{issue.id}', + f'bulk_verified_by={current_user.username}') + db.session.commit() + _flash_bulk(changed, skipped, 'verified and closed', + skip_reason='not awaiting verification') + + # ── Delete ─────────────────────────────────────────────────────────── + elif action == 'delete': + from app.utils import storage + photo_paths = [] + for issue in issues: + if issue.photo_path: + photo_paths.append(issue.photo_path) + for lst in (issue.mobile_photo_paths, issue.result_photos): + if lst: + photo_paths.extend(lst) + log_action(ACTION_DELETE, 'Issue', issue.id, f'#{issue.id}', + f'bulk deleted by {current_user.username}') + db.session.delete(issue) + changed += 1 + db.session.commit() + # Files go only after the rows are safely gone — a failure here leaves + # an orphaned file, which is recoverable; the reverse is not. + for rel_path in photo_paths: + storage.delete(rel_path) + _flash_bulk(changed, skipped, 'permanently deleted') + + logger.info('ISSUES | bulk | action=%s user=%s selected=%s changed=%s skipped=%s', + action, current_user.username, len(ids), changed, skipped) + return redirect(back) + + +def _flash_bulk(changed, skipped, verb, skip_reason='no change needed'): + """One consistent result message for every bulk action.""" + if not changed and not skipped: + flash('Nothing to do.', 'info') + return + parts = [f'{changed} issue{"s" if changed != 1 else ""} {verb}'] + if skipped: + parts.append(f'{skipped} skipped ({skip_reason})') + flash('. '.join(parts) + '.', 'success' if changed else 'warning') @bp.route('//request-verification', methods=['POST']) @@ -974,11 +1162,11 @@ def request_verification(issue_id): ) if not can_act: flash('Access denied.', 'danger') - return redirect(url_for('issues.view', issue_id=issue_id)) + return redirect(_view_url(issue_id)) if issue.status not in ('in_progress',): flash('Issue must be in progress to request verification.', 'warning') - return redirect(url_for('issues.view', issue_id=issue_id)) + return redirect(_view_url(issue_id)) issue.status = 'pending_verification' db.session.commit() @@ -1006,7 +1194,7 @@ def request_verification(issue_id): ) db.session.commit() flash('Issue marked as pending verification. Supervisors have been notified.', 'info') - return redirect(url_for('issues.view', issue_id=issue_id)) + return redirect(_view_url(issue_id)) # ── Verification queue ──────────────────────────────────────────────────────── @@ -1099,7 +1287,7 @@ def delete(issue_id): f'facility={facility_name}; description={issue_desc}') flash(f'Issue #{issue_id_snap} has been permanently deleted.', 'success') - return redirect(url_for('issues.index')) + return redirect(return_url(url_for('issues.index'))) # ── Quick-assign (AJAX) ─────────────────────────────────────────────────────── diff --git a/app/templates/inspections/list.html b/app/templates/inspections/list.html index b650e73..3951efa 100644 --- a/app/templates/inspections/list.html +++ b/app/templates/inspections/list.html @@ -107,10 +107,15 @@
{% if inspections.items %} + {% include 'partials/bulk_inspections_toolbar.html' %}
+ @@ -119,6 +124,11 @@ {% for ins in inspections.items %} + @@ -160,9 +170,9 @@
+ + #DateContractFacilityArea TemplateInspectorScore Status
+ + #{{ ins.id }} {{ ins.inspection_date.strftime('%Y-%m-%d %H:%M') }} {{ ins.facility.project.name if ins.facility and ins.facility.project else '—' }} {% if ins.status == 'in_progress' or ins.status == 'flagged' %} - Continue + Continue {% else %} - View + View {% endif %} {% if current_user.role in ['admin', 'director'] %} @@ -236,6 +247,7 @@ {% endblock %} {% block extra_js %} +{% include 'partials/bulk_select_js.html' %} diff --git a/app/utils/decorators.py b/app/utils/decorators.py index 66a23ef..926700c 100644 --- a/app/utils/decorators.py +++ b/app/utils/decorators.py @@ -28,6 +28,29 @@ def safe_redirect_url(url: str | None, fallback: str | None = None) -> str: return fallback return url + +def return_url(fallback: str) -> str: + """Where to go back to after a list-page action, preserving its filters. + + Reads the `next` value the page carried through the action — POST body + first (forms), then query string (links) — and validates it with + safe_redirect_url, so a crafted `next` can never redirect off-site. + + The problem this solves: a delete or an edit launched from a filtered list + used to redirect to the bare index, throwing away the filters the user had + set. Every list-page action now round-trips the list URL instead. + + `next` is deliberately the FULL list URL (page number and all), not a + reconstructed set of arguments — that keeps this helper working when a new + filter is added to either list page without anyone having to remember to + thread it through here. + """ + from flask import request + return safe_redirect_url( + request.form.get('next') or request.args.get('next'), + fallback=fallback, + ) + def admin_required(f): @wraps(f) def decorated_function(*args, **kwargs):