HH/reports/feedback-modules-qa-report.md

# Feedback Modules QA Audit Report

**Modules:** Complaint · Inquiry · Observation · Suggestion · Appreciation
**Date:** 2026-06-14
**Environment:** Local dev (`runserver` :8000 → PostgreSQL `px360_db`), headed Chromium (`DISPLAY=:0`), console email backend, isolated **E2E Test Hospital** mirroring **HH-N (Al Nuzha)** hierarchy.
**Method:** Exploratory, run-to-completion Playwright spec (`e2e/tests/workflows/feedback-modules-audit.spec.ts`); every step guarded; console/page-error/HTTP-error listeners captured observations into `e2e/results/audit-observations.json`. Roles tested deeply: `px_admin, hospital_admin, dept_manager, px_employee, staff`; access-reachability for all 10 roles.

## Headline numbers (clean run, server stable)
- **26/26 audit tests completed** (0 hard failures). 8.1 min.
- **124 observations:** 50 PASS · 49 WARN · 10 INFO · 5 SKIP · **0 FAIL (server up)**
- Baseline lifecycle specs: inquiry **7/7 pass**, observation **5/6 pass**, complaint **0/10** (suite broken — see §5).

---

## 1. Complaint
**Routes:** public `/complaints/public/submit/`, list `/complaints/`, detail `/complaints/<id>/`, track `/complaints/public/track/`, export `/complaints/export/csv/`.

✅ **Handled correctly**
- Public form submits successfully and creates a `Complaint` (verified in DB: 4 records in E2E-HOSP).
- Authenticated list renders table + status badges for `hospital_admin, dept_manager, px_employee, staff` (24 badges each).
- Detail page opens; **add-note** posts successfully; **CSV export** returns `200 text/csv` (6 lines, valid header).
- Authenticated create form (`/complaints/new/`) renders.

⚠️ **Issues found**
- **Reference number not surfaced/extractable** on the public success page — the test couldn't capture a `CMP-…` ref from body or URL, so the subsequent **track test ran with a dummy ref**.
- **`activate` action did not reflect `in_progress`** in the page text after submit (`status text present: false`).
- `px_admin` list renders **no table** (see cross-cutting §4.1).
- One `net::ERR_ABORTED` on a detail navigation (test re-clicked; benign).

🔧 **Improvements**
- Display the reference number prominently on the public success page (and in the success URL) so submitters can track.
- Verify the activate/self-assign flow updates the visible status badge immediately.

## 2. Inquiry
**Routes:** public `/inquiries/public/submit/`, list `/inquiries/`, detail `/inquiries/<id>/`.

✅ **Handled correctly**
- Public form submits and creates an `Inquiry` (3 records in E2E-HOSP).
- List + detail open for `hospital_admin, dept_manager, px_employee, staff`.
- **Respond modal opens** for `hospital_admin` (form present, closeable).
- Status badges render.

⚠️ **Issues found**
- **Respond action only visible to `hospital_admin`** — `dept_manager`, `px_employee`, `staff` see **no respond button** on inquiry detail. Confirm whether this is intended RBAC or a gap.
- Public submit **did not surface an `INQ-…` reference** (regex didn't match the success body) — minor.
- `px_admin` list renders **no table** (§4.1).

🔧 **Improvements**
- Clarify/extend inquiry "respond" permissions to the roles expected to action inquiries.
- Show the inquiry reference on the public success page.

## 3. Observation
**Routes:** public `/observations/new/`, success `/observations/submitted/<code>/`, track `/observations/track/`, list `/observations/`, detail `/observations/<id>/`.

✅ **Handled correctly**
- Public form **renders**, and a valid POST (with `incident_datetime` in `YYYY-MM-DD HH:MM:SS`) returns **302** with a tracking code (e.g. `OBS-R208ER`) — **3 records created** in E2E-HOSP.
- Public **track page** responds.
- List + detail open for `hospital_admin, dept_manager, px_employee, staff` (all pass).

⚠️ **Issues found**
- **UI cascade is fragile & format-sensitive:** the public form's `hospital → location_type → area → category → department → section` is pure AJAX; submission silently **re-renders (200, no redirect)** unless `incident_datetime` includes **seconds** (`2026-06-13 12:00` fails, `2026-06-13 12:00:00` works). A real user using the date picker is fine, but raw entry is unforgiving.
- `px_admin` list renders **no table** (§4.1).

🔧 **Improvements**
- Add a proper datetime picker so the canonical format is always sent.
- Show client-side validation errors clearly when the cascade fields are incomplete.

## 4. Suggestion (`/suggestions/`, feedback app)
**Routes:** public `/suggestions/public/suggestion/` (**POST-only**), list `/suggestions/`, create `/suggestions/create/`, detail `/suggestions/<id>/`.

✅ **Handled correctly**
- Authenticated list renders for `hospital_admin, dept_manager, px_employee, staff`.
- Public endpoint correctly returns **405 on GET** (POST-only) and **400 on empty POST** — i.e. the endpoint is wired and validating.

⚠️ **Issues found (highest-impact)**
- **No public GET form** for suggestions — anonymous users have **no UI** to submit a suggestion (only a POST endpoint). Inconsistent with Complaint/Inquiry/Observation which all have public GET forms.
- **Authenticated create did NOT persist a record.** The test reported a "success indicator" but **0 `Feedback` rows exist in E2E-HOSP** (only 2 stale rows in HH-N from prior manual testing). The create form (`feedback_form.html`) fields are `comm_req, contact_name, contact_phone, hospital(select), message` — **no `title`, `category`, or `feedback_type`** inputs, yet the test (and the `FeedbackForm` ModelForm) assume them. The custom `feedback_create` view reads only those 5 fields. → The authenticated suggestion-create flow needs manual verification; the automated harness could not reliably drive it.

🔧 **Improvements**
- Add a public GET form for suggestions (parity with other modules), or document that suggestions are internal-only.
- Reconcile the create form's actual fields with the `FeedbackForm` ModelForm (title/category/feedback_type are model fields but absent from the create UI) and verify an authenticated user can actually save a suggestion end-to-end.

## 5. Appreciation (`/appreciation/`)
**Routes:** public `/appreciation/public/submit/` (**JSON POST**), list `/appreciation/`, detail `/appreciation/detail/<id>/`, activate `/appreciation/detail/<id>/activate/`, send `/appreciation/detail/<id>/send/`, acknowledge `/appreciation/acknowledge/<id>/`.

✅ **Handled correctly**
- **Public JSON POST creates a DRAFT** (`APR-…` ref returned, CSRF handled) — 2 records in E2E-HOSP.
- List + detail open for `hospital_admin`.
- **Full internal workflow works:** `activate` (select staff → activated, AI analysis triggered) → `send` (status → sent, notification dispatched to console). ✅

⚠️ **Issues found**
- **No public GET form** — like suggestions, appreciation public submission is **POST-only** (GET → 405). No anonymous-facing UI.
- `acknowledge` was **not exercised** (only the recipient can acknowledge; out of scope for a non-recipient session).
- One `net::ERR_ABORTED` on a detail navigation (benign test artifact).

🔧 **Improvements**
- Add a public GET form (parity), or document that appreciation submission is JSON-API-only.
- Add an automated path to test `acknowledge` (login as the recipient staff's user).

---

## 6. Cross-cutting findings (prioritized)

### 🔴 P1 — `px_admin` sees empty lists across ALL modules
For Complaint, Inquiry, Observation, and Suggestion, `px_admin` (confirmed logged in, scoped to E2E Test Hospital, `is_px_admin()=True`) renders **no data table**, while every other role sees populated tables. Records demonstrably exist in E2E-HOSP. This points to the `tenant_hospital` session-scoping logic for PX Admins not resolving E2E-HOSP (the list views branch on `if user.is_px_admin(): if selected_hospital: …`). **Needs investigation** — PX Admin is the primary oversight role and currently appears to get a blank view.

### 🔴 P1 — Suggestion authenticated create doesn't persist
See §4. No `Feedback` record is saved from the authenticated create flow. The create form's fields don't match the model/form assumptions.

### 🟡 P2 — Inconsistent public-form parity
| Module | Public GET form | Public submit |
|--------|:--:|:--:|
| Complaint | ✅ | ✅ |
| Inquiry | ✅ | ✅ |
| Observation | ✅ | ✅ |
| Suggestion | ❌ (POST-only) | ⚠️ endpoint exists |
| Appreciation | ❌ (POST-only) | ⚠️ JSON-only |

Anonymous UX is inconsistent. Either add GET forms for the last two or document them as internal/API-only.

### 🟡 P2 — Inconsistent reference-number formats
`CMP-YYYYMMDD-NNNNNN`, `INQ-<8 hex>`, `OBS-<6 alnum>`, `APR-<8 hex>`. Consider a unified scheme for tracking/UX consistency.

### 🟢 P3 — Reference numbers not surfaced on success pages
Complaint & Inquiry public success pages don't expose the reference in an easily extractable way (affects tracking UX and the public track flow).

### 🟢 P3 — Inquiry "respond" limited to `hospital_admin`
Other actioning roles (`dept_manager`, `px_employee`, `staff`) see no respond button. Confirm intent.

### 🟢 P3 — Observation public form date format & cascade fragility
Requires `incident_datetime` with seconds; cascade gives no inline validation feedback.

### ⚪ Ops — Dev server stability under automation
During one run the `runserver` autoreloader **restarted mid-suite** (connection reset → refused), invalidating that pass. For repeatable automation, run a dedicated QA server with `--noreload` (or a gunicorn/uvicorn instance) instead of the autoreloading dev server.

---

## 7. Role / access matrix (observed)
All 10 roles were able to **reach** every module root (no incorrect login-redirect blocks). The audit's PASS/WARN labels for the matrix are spurious (a comparison-flaw in the exploratory status logic: `onLogin === expected` mixed "blocked?" with "expected-allowed?"), so the matrix should be treated as **inconclusive for fine-grained RBAC**. The documented `e2e/tests/roles/access-matrix.spec.ts` remains the source of truth for reachability; this audit did not contradict it. **Recommended:** fix the audit's matrix status comparison and re-run for a clean RBAC signal.

---

## 8. Baseline lifecycle comparison (existing specs)
| Spec | Result | Notes |
|------|--------|-------|
| `complaint-lifecycle.spec.ts` | **0/10 run** | BROKEN — uses outdated `#id_complainant_name` / `#id_…` selectors; the public complaint form now exposes fields by `name=` only. Test 1 fails → 9 skipped (serial). **Update selectors.** |
| `inquiry-lifecycle.spec.ts` | **7/7 pass** | ✅ Healthy. |
| `observation-lifecycle.spec.ts` | **5/6 pass** | 1 fail: "severity badges render correctly". Public submit works here. |

The complaint-lifecycle staleness is itself a finding — the public complaint form's field IDs were removed but the test wasn't updated.

---

## 9. What was changed for this audit (all reversible)
- **New:** `apps/core/management/commands/create_e2e_isolated_env.py` — builds isolated E2E-HOSP (mirrors HH-N: 27 Areas, 37 Departments, 171 Sections, 1 SubSection) + 10 role users.
- **New:** `e2e/tests/workflows/feedback-modules-audit.spec.ts` + `e2e/helpers/audit.ts`.
- **Edited:** `.env` → `EMAIL_BACKEND=console` (backup at `.env.bak_qa`); `playwright.config.ts` → env-overridable `timeout/actionTimeout/navigationTimeout/slowMo` (defaults preserve prior behavior).
- **Created:** isolated hospital `E2E-HOSP` (kept for future QA), org `E2E-ORG`, source `E2E-TEST`, 10 `e2e-*@px360.test` users (password `Dev@123456`), plus audit-generated records inside E2E-HOSP.
- No migrations, no commits.

## 10. Reproduce
```bash
# 1. (one-time) build the isolated sandbox mirroring HH-N
uv run manage.py create_e2e_isolated_env --delete-existing
# 2. email -> console (already applied; .env.bak_qa has the SMTP original)
# 3. run the audit headed
E2E_TIMEOUT=120000 E2E_ACTION_TIMEOUT=12000 E2E_NAV_TIMEOUT=20000 E2E_SLOWMO=60 \
  npx playwright test e2e/tests/workflows/feedback-modules-audit.spec.ts --workers=1 --headed
# 4. observations + report inputs
cat e2e/results/audit-observations.json
```

## 11. Recommended next steps (for these 5 modules)
1. Fix P1 `px_admin` empty-list scoping.
2. Fix P1 suggestion authenticated create persistence; reconcile form fields.
3. Decide public-form parity for suggestion & appreciation (add GET forms or document as internal).
4. Update `complaint-lifecycle.spec.ts` selectors (`name=` instead of `#id_`).
5. Surface reference numbers on all public success pages.
6. Re-run this audit after fixes (sandbox is retained); then proceed to the next module group.

---

# 12. Champion / Manager Workflow Audit (send-to-department lifecycle)

Covers the full multi-actor loop the original audit missed: **PX-team sends → champion investigates → manager reviews → PX-team accepts → resolve**, plus the token investigation sub-flow and both rejection loops. Spec: `e2e/tests/workflows/champion-manager-workflow.spec.ts`. State asserted after every step via the `get_e2e_workflow_state` CLI helper (Node can't read the DB).

**Result: 4/4 flows pass. 32 observations: 26 PASS · 1 FAIL · 3 WARN · 2 INFO.** Two real bugs were found and fixed during the run.

## ✅ Handled correctly (verified end-to-end)
| Flow | Path | Result |
|------|------|--------|
| **A — happy path** | PX send → champion response → manager **approve** → PX **accept** → resolve | ✅ all 11 state transitions correct (`sent` → `response_submitted+pending` → `approved` → `acceptable` → `resolved`) |
| **B — token investigation** | champion creates Qs (token) → staff answers (token) → champion reviews + writes response | ✅ `response_submitted=True`, `manager_review_status=pending` |
| **C1 — manager reject loop** | manager rejects → response cleared + `response_submitted=False` → champion re-responds → approve → accept → resolve | ✅ reject correctly returns the complaint to the champion |
| **C2 — PX not-acceptable loop** | PX marks not-acceptable → response cleared → champion re-responds → approve → accept → resolve | ✅ correctly returns to champion |

The `ComplaintInvolvedDepartment` state machine behaves exactly as designed: `sent → response_submitted(pending) → approved → acceptable`, and both rejection branches reset `response_submitted=False` + clear the response, sending it back to the champion.

## 🔧 Bugs found & fixed during this audit
1. **`NameError: name 'InvestigationAnswer' is not defined`** at `apps/complaints/views.py:4021` — `champion_start_investigation` used `InvestigationAnswer` without importing it, so the champion's "create questions" POST returned **500** (DB writes before the crash still persisted, masking the error). **Fixed:** added `InvestigationAnswer` to the view's imports.
2. **`AttributeError` waiting to happen** at `apps/complaints/views.py:4064` — `staff_member.phone_number`, but the `Staff` model field is `phone`. Would have crashed the moment the NameError above was fixed. **Fixed:** `phone_number` → `phone`.

## ⚠️ Remaining issues
- **`ReferenceError: lucide is not defined`** (frontend, Flow B token pages) — the icon library (`lucide`) isn't loaded on the investigation templates (`investigation_questions.html` / `investigation_respond.html` / `investigation_review.html`). Cosmetic only; the forms still work. Fix: include the lucide script on those templates (or use the icon partial the rest of the app uses).
- **`GET /organizations/departments/<id>/analytics/` → ERR_ABORTED** (Flows C1/C2) — the department-analytics XHR on the dept pages fails (likely 404/500). Minor; doesn't block the workflow.
- **SMS gateway (Mshastra) "IP address not allowed"** — external provider config, not app code; expected in dev.

## 🔩 Test harness added
- `apps/core/management/commands/create_e2e_isolated_env.py` extended — binds `e2e-dept-manager` as `department.manager` and creates a `Staff` profile for `e2e-staff` in the champion's department (Contact Center), so all three workflow roles exist.
- `apps/core/management/commands/seed_e2e_complaint.py` (new) — seeds an open complaint + accused staff, prints ids.
- `apps/core/management/commands/get_e2e_workflow_state.py` (new) — prints the full workflow state (idept fields + tokens) for assertions.
- `e2e/tests/workflows/champion-manager-workflow.spec.ts` (new) — Flows A/B/C1/C2, drives the real HTTP endpoints (AJAX + form POSTs), asserts state after each step.

## Reproduce
```bash
uv run manage.py create_e2e_isolated_env --delete-existing
E2E_TIMEOUT=120000 E2E_ACTION_TIMEOUT=12000 E2E_NAV_TIMEOUT=20000 \
  npx playwright test e2e/tests/workflows/champion-manager-workflow.spec.ts --workers=1 --headed
```

---

# 13. Inquiry & Observation dept-response workflow (simpler flow)

These two modules use a **simpler** flow than complaints: **PX-team sends to department → champion responds → PX reviews (accept/reject)**. No manager review, no investigation questions. State is flat fields on the `Inquiry`/`Observation` model (no join table). Spec: `e2e/tests/workflows/inquiry-observation-dept-workflow.spec.ts`, 6 flows (3 per module), driven via the real HTTP endpoints with state asserted via `get_e2e_dept_response_state`.

**Result: 6/6 flows pass. 59 observations: 50 PASS · 5 WARN · 0 FAIL.**

## ✅ Handled correctly (verified)
| Flow | Inquiry | Observation |
|------|:--:|:--:|
| **A — happy path** (send → champion response → PX accept → resolve) | ✅ | ✅ (resolve best-effort — see below) |
| **B — token path** (send mints token → champion token response → PX accept) | ✅ backend | ✅ backend |
| **C — reject loop** (PX not-acceptable → response cleared → champion re-responds → accept → resolve) | ✅ | ✅ |

State machines behave correctly: send sets `transferred_to_department` (inquiry) / `sent_to_department=True` (observation); champion response sets `department_response_en` + `department_responded_at` + `dept_response_acceptance_status=pending`; PX `acceptable` → `acceptance_status=acceptable`; PX `not_acceptable` → **clears the response** and returns it to the champion (verified — `response_en_set=False` after reject). Inquiry resolve requires a PX `response` first (`inquiry_respond`) — handled.

## ⚠️ Issues found
1. **Token-response form page is broken (both modules).** GET `/inquiries/<id>/respond/<token>/` and `/observations/<id>/respond/<token>/` render the **dashboard chrome with no response form** (no `response_en` textarea) instead of `inquiry_response_form_token.html` / `response_form_token.html`. The backend POST still accepts the token response, but a **truly anonymous visitor can't submit** (the broken page renders no `{% csrf_token %}`, so they get a 403 on POST). High-impact for the email-link UX since champions receive this link by email. Reproduced via Django test client (GET returns a 13176-char page titled "PX360 Dashboard - Blue Edition" with no `<form>`).
2. **`NameError: get_email_header_html` during inquiry transfer** (`inquiry_transfer_to_department`) — the department-assigned notification email silently fails ("Failed to send department notification: name 'get_email_header_html' is not defined"). Caught/logged, so the transfer succeeds, but the notification isn't sent.
3. **Observation can't jump `new → resolved`** — its status machine requires intermediate steps (new → triaged/assigned/in_progress → resolved). `observation_change_status` accepts the POST (302) but the status stays `new`. (Reported as WARN, not FAIL — resolution is outside the dept-response flow's core.)

## 🔧 Improvements recommended
- **Fix the token-response form pages** so they render the `response_en`/`response_ar` form (and a csrf token) — the templates exist and are correct; the view/template-wiring is returning the dashboard instead. This unblocks the email-link response path for champions.
- **Fix `get_email_header_html` import** in `inquiry_transfer_to_department` (mirror the fix pattern used elsewhere, e.g. complaints).
- Confirm whether observation resolution should be reachable directly from the dept-response flow (add an `activate`-equivalent or allow `new → resolved` if appropriate).

## 🔩 Test harness added
- `apps/core/management/commands/seed_e2e_dept_response.py` — seeds an open inquiry/observation, prints ids.
- `apps/core/management/commands/get_e2e_dept_response_state.py` — prints the dept-response state + token for assertions.
- `playwright.config.ts` — `E2E_MAXIMIZED=1` launches the browser maximized/fullscreen (viewport=null + `--start-maximized`).

## Reproduce
```bash
E2E_MAXIMIZED=1 E2E_TIMEOUT=120000 E2E_ACTION_TIMEOUT=12000 E2E_NAV_TIMEOUT=20000 E2E_SLOWMO=70 \
  npx playwright test e2e/tests/workflows/inquiry-observation-dept-workflow.spec.ts --workers=1 --headed
```