fix(review): T&E review shows why the database rejected a row (#190)

api/services/review.py

@@ -31,18 +31,24 @@ def _s():
 
 def _upload(cur, upload_id: int) -> dict:
     cur.execute(sql.SQL(
-        "SELECT u.file_name, u.columns, u.column_types, h.cells, "
+        "SELECT u.file_name, u.columns, u.column_types, h.cells, u.mode, u.row_errors, "
         "  (SELECT count(*) FROM {s}.source_rows r WHERE r.upload_id = u.id AND r.row_number > 0) "
         "FROM {s}.uploads u LEFT JOIN {s}.source_rows h ON h.upload_id = u.id AND h.row_number = 0 "
         "WHERE u.id = %s").format(s=_s()), (upload_id,))
     row = cur.fetchone()
     if row is None:
         raise ReviewError(404, "No such upload")
-    name, columns, types, header, total = row
+    name, columns, types, header, mode, stored_errors, total = row
     columns = columns or sanitize_columns(header or [])
-    # Uploads stored before 0008, or in T&E mode, have no types: every value counts as text.
+    # Uploads stored before 0008, or T&E uploads before #190, have no types: every value counts as text.
     types = types if types and len(types) == len(columns) else ["text"] * len(columns)
-    return {"fileName": name, "columns": columns, "types": types, "total": total}
+    # #190: T&E rows the database rejected (CHECK, FK, UNIQUE, ...). Errors with a column are
+    # type failures, which _validate finds again; these carry only the database's reason.
+    # ponytail: stored list is capped at MAX_ROW_ERRORS_REPORTED, rows past the cap show no
+    # database reason; edited rows aren't re-checked against constraints until M6's import.
+    db_errors = {e["rowNumber"]: e["reason"] for e in (stored_errors or [])
+                 if mode == "te" and "column" not in e and "rowNumber" in e}
+    return {"fileName": name, "columns": columns, "types": types, "total": total, "_dbErrors": db_errors}
 
 
 def _validate(cells: list[str], raw: list[str], columns: list[str], types: list[str]) -> list[dict]:
@@ -92,12 +98,15 @@ def _rows(cur, upload_id: int, meta: dict, numbers: list[int] | None = None,
         original = (list(raw) + [""] * width)[:width]
         current = [edits.get(n, {}).get(c, v) for c, v in enumerate(original)]
         decision = decisions.get(n)
+        errors = _validate(current, raw, meta["columns"], meta["types"])
+        if n in meta["_dbErrors"] and n not in edits:
+            errors.append({"column": "(database)", "value": "", "reason": meta["_dbErrors"][n]})
         out.append({
             "rowNumber": n, "original": original, "current": current,
             "editedColumns": sorted(edits.get(n, {})),
             "decision": decision["decision"] if decision else "include",
             "decisionReason": decision["reason"] if decision else None,
-            "errors": _validate(current, raw, meta["columns"], meta["types"]),
+            "errors": errors,
         })
     return out
 
@@ -111,7 +120,8 @@ def page(cur, upload_id: int, offset: int, limit: int) -> dict:
         "       (SELECT count(DISTINCT row_number) FROM {s}.cell_edits WHERE upload_id = %s)").format(s=_s()),
         (upload_id, upload_id))
     excluded, edited = cur.fetchone()
-    return {**meta, "offset": offset, "limit": limit, "excludedRows": excluded, "editedRows": edited,
+    public = {k: v for k, v in meta.items() if k != "_dbErrors"}
+    return {**public, "offset": offset, "limit": limit, "excludedRows": excluded, "editedRows": edited,
             "rows": _rows(cur, upload_id, meta, offset=offset, limit=limit)}
 
 

api/services/te_loader.py

@@ -332,7 +332,9 @@ def _err(row_number: int, column: str, value: str, reason: str) -> None:
         "duplicateRowsSkipped": 0,
         "failedRows": failed_row_count,
         "columns": columns,
-        "types": [],
+        # #190: stored with the upload so review revalidates against the target's types.
+        # Columns Postgres casts itself (uuid, enums, ...) review as text.
+        "types": [t or "text" for t in col_types],
         "rowErrors": row_errors,
         "logs": logs,
     }

csv-table-hub-main/src/__tests__/review-table.test.tsx

@@ -32,7 +32,11 @@ const page = {
     row(2, ["x", "b"], {
       errors: [{ column: "n", value: "x", reason: '"x" is not a whole number' }],
     }),
-    row(3, ["3", "c"], { decision: "exclude", decisionReason: "duplicate of row 1" }),
+    row(3, ["3", "c"], {
+      decision: "exclude",
+      decisionReason: "duplicate of row 1",
+      errors: [{ column: "(database)", value: "", reason: "violates check constraint" }],
+    }),
   ],
 };
 
@@ -82,6 +86,8 @@ it("shows every row with its validation, edits and decision", async () => {
   );
   expect(screen.getByText("Invalid")).toBeInTheDocument();
   expect(screen.getByText("Excluded")).toHaveAttribute("title", "duplicate of row 1");
+  // #190: a reason not tied to a column (the database rejected the row) is shown beside it.
+  expect(screen.getByText("violates check constraint")).toBeInTheDocument();
 });
 
 it("a reviewer excludes a row with a required reason", async () => {

csv-table-hub-main/src/components/review-table.tsx

@@ -161,7 +161,8 @@ function ReviewLine({
 }) {
   const excluded = row.decision === "exclude";
   const errorFor = (name: string) => row.errors.find((e) => e.column === name);
-  const extra = errorFor("(beyond header)");
+  // Reasons not tied to a column: "(beyond header)" cells, or "(database)" for a row the target rejected (#190).
+  const extra = row.errors.find((e) => !columns.includes(e.column));
   return (
     <TableRow className={excluded ? "opacity-50" : undefined}>
       <TableCell className="text-right tabular-nums text-xs text-muted-foreground">

docs/test-cases/M4-review.md

@@ -88,7 +88,7 @@ $u = $up.uploadId
 
 ## Not covered here
 - **The review screen** (M4b, PR #143) has its own component tests (`review-table.test.tsx`) and the browser check `scripts/e2e/m4-review.js`.
-- **T&E-mode uploads** store no column types, so their values are reviewed as text with no type checks. That's a known limit, noted on #142.
+- **T&E-mode uploads** store the target table's column types (#190), and review shows the database's reason for rows it rejected (CHECK, FK, UNIQUE) until the row is edited. Edited T&E rows are re-checked against types only; constraints are checked at M6's import. Uploads made before #190 have no stored types and review as text.
 - **Applying review to the import** belongs to M6 (#110). It isn't tested here, because it isn't built yet.
 
 ## Record of runs

tests/test_api_coverage.py

@@ -389,7 +389,8 @@ def test_response_shape_matches_contract(self):
                     "columns", "rowErrors"):
             self.assertIn(key, body, f"missing key '{key}'")
         self.assertEqual(body["duplicateRowsSkipped"], 0)
-        self.assertEqual(body["types"], [])
+        # #190: one reviewable type per column (organisations' columns are all text).
+        self.assertEqual(body["types"], ["text"] * len(body["columns"]))
 
     def test_column_mismatch_returns_error(self):
         csv = "bogus_col,fake_col\nval1,val2\n"

tests/test_review.py

@@ -109,6 +109,30 @@ def test_refusals(self):
                 self.assertEqual(res.status_code, code)
         self.assertEqual(self.client.get(f"/api/csv/uploads/{uid}/rows/2/history").json(), [])
 
+    def test_te_review_shows_the_database_reason_a_row_was_rejected(self):
+        # #190: T&E uploads stored no types, so review said errors: [] for a row the CHECK rejected.
+        good, bad = f"TE_REV_{self.tag}_ok", f"TE_REV_{self.tag}_bad"
+        res = self.client.post("/api/csv/upload", json={
+            "fileName": f"te_rev_{self.tag}.csv", "mode": "te", "targetTable": "organisations",
+            "content": f"name,org_type,country\n{good},government,AU\n{bad},not_a_type,AU\n"}).json()
+        try:
+            self.assertEqual(res["failedRows"], 1, res)
+            page = self._rows(res["uploadId"])
+            self.assertEqual(len(page["types"]), 3)
+            ok_row, bad_row = page["rows"]
+            self.assertEqual(ok_row["errors"], [])
+            [err] = bad_row["errors"]
+            self.assertEqual(err["column"], "(database)")
+            self.assertIn("org_type", err["reason"])
+            # Once a reviewer edits the row, the upload-time reason no longer describes it.
+            self.assertEqual(self._edit(res["uploadId"], 2, "org_type", "prime").status_code, 200)
+            self.assertEqual(self._rows(res["uploadId"])["rows"][1]["errors"], [])
+        finally:
+            with Conn() as conn, conn.cursor() as cur:
+                cur.execute(sql.SQL("DELETE FROM {}.organisations WHERE name = ANY(%s)").format(
+                    sql.Identifier(settings.TE_SCHEMA)), ([good, bad],))
+                conn.commit()
+
     def test_changes_are_audited_and_insert_only(self):
         uid = self._three_rows()
         self._decide(uid, 3, "exclude")