backend updates for bulk product uploads from user

2026-08-17 15:23:10 +05:30
parent 5e373ea20c
commit dba8d36175
7 changed files with 1290 additions and 218 deletions
--- a/tests/test_user_products_upload.py
+++ b/tests/test_user_products_upload.py
@@ -0,0 +1,370 @@
+"""
+Tests for the user product upload path.
+
+The bug these exist for: uploading a spreadsheet returned a success message
+while nothing reached the database. So the assertions here are deliberately not
+"did it answer 2xx" - they are "did it answer 2xx *and* hand the rows to the
+store", and, for every failure mode, "did it refuse to call that a success".
+
+Hermetic, like the rest of the suite: the store, the embedding model and the
+seed-catalog writer are all substituted, so nothing here touches a real
+database, downloads a model, or writes into data/seed_catalogs/.
+"""
+from __future__ import annotations
+
+import io
+
+import pytest
+
+from app.api.routers import user_products
+from app.services.vector_store import VectorStoreUnavailable
+
+UPLOAD_URL = "/api/user/products/upload-file"
+
+# The headers the frontend's "Download Sample CSV" button produces.
+TEMPLATE_CSV = (
+    "Brand Name,Product Name / Variant,Category,Price Range,Final Price (₹),"
+    "Barcode (GTIN/EAN),HSN Code,Custom Image URL,Description\n"
+    "Lion Dates,Lion Dates 450g,Health Foods,₹160-220,185.00,20086040,2008,,Premium dates\n"
+    "Naga,Naga Maida 2kg,Flour & Grains,₹90-110,98.00,8906012345001,1101,,Refined wheat flour\n"
+)
+
+
+@pytest.fixture
+def store(monkeypatch):
+    """Capture what the endpoint hands to the database instead of writing it.
+
+    Also stubs the two slow collaborators. `embed_texts` would download and load
+    a sentence-transformer; `get_products_by_brand` would open a real connection
+    to whatever DB_HOST points at.
+    """
+    calls = []
+
+    def fake_upsert(brand, products, cleanup=False):
+        calls.append((brand, products))
+        return len(products)
+
+    monkeypatch.setattr(user_products, "upsert_brand_products", fake_upsert)
+    monkeypatch.setattr(user_products, "embed_texts", lambda texts: [[0.0] * 384 for _ in texts])
+    monkeypatch.setattr(user_products, "get_products_by_brand", lambda *a, **k: [])
+    monkeypatch.setattr(user_products, "upsert_products_into_catalog_file", lambda brand, products: None)
+    return calls
+
+
+def _upload(client, headers, content: str | bytes, name: str = "products.csv"):
+    data = content.encode("utf-8") if isinstance(content, str) else content
+    return client.post(UPLOAD_URL, files={"file": (name, data)}, headers=headers)
+
+
+# ---------------------------------------------------------------------------
+# The reported bug: success without persistence
+# ---------------------------------------------------------------------------
+def test_upload_is_not_a_success_when_the_database_is_unreachable(client, user_headers, monkeypatch, store):
+    """The exact production symptom. An unreachable database must not answer 201."""
+    def unavailable(brand, products, cleanup=False):
+        raise VectorStoreUnavailable("the product database is unreachable (host=db:5432)")
+
+    monkeypatch.setattr(user_products, "upsert_brand_products", unavailable)
+
+    resp = _upload(client, user_headers, TEMPLATE_CSV)
+
+    assert resp.status_code == 503, resp.text
+    detail = resp.json()["detail"]
+    assert "Nothing was saved" in detail
+    assert "unreachable" in detail
+
+
+def test_single_add_is_not_a_success_when_the_database_is_unreachable(client, user_headers, monkeypatch, store):
+    def unavailable(brand, products, cleanup=False):
+        raise VectorStoreUnavailable("the product database is unreachable (host=db:5432)")
+
+    monkeypatch.setattr(user_products, "upsert_brand_products", unavailable)
+
+    resp = client.post(
+        "/api/user/products/add",
+        json={"brand": "Lion Dates", "product_name": "Lion Dates 450g"},
+        headers=user_headers,
+    )
+
+    assert resp.status_code == 503, resp.text
+    assert "not saved" in resp.json()["detail"]
+
+
+def test_upload_reaches_the_database_and_reports_what_it_stored(client, user_headers, store):
+    resp = _upload(client, user_headers, TEMPLATE_CSV)
+
+    assert resp.status_code == 201, resp.text
+    body = resp.json()
+    assert body["status"] == "success"
+    assert body["added_count"] == 2
+    assert body["error_count"] == 0
+    assert body["rows_total"] == 2
+
+    stored = {p["product_name"]: p for _, products in store for p in products}
+    assert set(stored) == {"Lion Dates 450g", "Naga Maida 2kg"}
+
+    dates = stored["Lion Dates 450g"]
+    assert dates["category"] == "Health Foods"
+    assert dates["final_selling_price"] == 185.0
+    assert dates["price_range"] == "₹160-220"
+    # Not "20086040.0" - an identifier read as a float and stringified is a
+    # different identifier.
+    assert dates["barcode"] == "20086040"
+    assert dates["hsn_code"] == "2008"
+    assert stored["Naga Maida 2kg"]["barcode"] == "8906012345001"
+
+
+def test_a_real_xlsx_workbook_imports_with_its_identifiers_intact(client, user_headers, store):
+    """The reported case was an .xlsx upload, and Excel is where identifiers rot.
+
+    A 13-digit barcode in a spreadsheet cell is a number to Excel, so it arrives
+    as a float and stringifies to "8906012345001.0"; an HSN code of 0402 loses
+    its leading zero. Both are then stored - silently - as a different value
+    than the one in the file.
+    """
+    openpyxl = pytest.importorskip("openpyxl", reason="declared in requirements.txt for .xlsx uploads")
+
+    workbook = openpyxl.Workbook()
+    sheet = workbook.active
+    sheet.append(["Brand Name", "Product Name", "Final Price (₹)", "Barcode (GTIN/EAN)", "HSN Code"])
+    sheet.append(["Lion Dates", "Lion Dates 450g", 185, 8906012345001, "0402"])
+
+    buffer = io.BytesIO()
+    workbook.save(buffer)
+
+    resp = _upload(client, user_headers, buffer.getvalue(), name="products.xlsx")
+
+    assert resp.status_code == 201, resp.text
+    assert resp.json()["added_count"] == 1
+
+    stored = [p for _, products in store for p in products][0]
+    assert stored["barcode"] == "8906012345001"
+    assert stored["hsn_code"] == "0402"
+    assert stored["final_selling_price"] == 185.0
+
+
+def test_partial_import_is_reported_as_partial_not_success(client, user_headers, monkeypatch, store):
+    """One brand failing must neither sink the other nor be called a success."""
+    def selective(brand, products, cleanup=False):
+        if "naga" in brand.lower():
+            raise RuntimeError("column overflow on selling_price")
+        return len(products)
+
+    monkeypatch.setattr(user_products, "upsert_brand_products", selective)
+
+    resp = _upload(client, user_headers, TEMPLATE_CSV)
+
+    assert resp.status_code == 201, resp.text
+    body = resp.json()
+    assert body["status"] == "partial"
+    assert body["added_count"] == 1
+    assert body["error_count"] == 1
+    assert body["errors"][0]["row"] == 3          # header is row 1, Naga is row 3
+    assert "column overflow" in body["errors"][0]["error"]
+
+
+# ---------------------------------------------------------------------------
+# Header handling
+# ---------------------------------------------------------------------------
+def test_template_headers_all_map_to_their_field():
+    mapping = user_products.map_spreadsheet_columns([
+        "Brand Name", "Product Name / Variant", "Category", "Price Range",
+        "Final Price (₹)", "Barcode (GTIN/EAN)", "HSN Code", "Custom Image URL",
+        "Description",
+    ])
+
+    assert mapping.columns == {
+        "brand": "Brand Name",
+        "product_name": "Product Name / Variant",
+        "category": "Category",
+        "price_range": "Price Range",
+        "final_selling_price": "Final Price (₹)",
+        "barcode": "Barcode (GTIN/EAN)",
+        "hsn_code": "HSN Code",
+        "image_url": "Custom Image URL",
+        "description": "Description",
+    }
+    assert mapping.ignored == []
+
+
+def test_a_product_sku_column_is_not_mistaken_for_the_product_name():
+    """'Product SKU' contains 'product'. Matched loosely, it used to become a
+    second product_name column, and duplicate columns are what turned a row
+    into a stringified Series."""
+    mapping = user_products.map_spreadsheet_columns(["Brand", "Product Name", "Product SKU"])
+
+    assert mapping.columns["product_name"] == "Product Name"
+    assert mapping.columns["product_sku"] == "Product SKU"
+
+
+def test_colliding_columns_are_reported_and_do_not_corrupt_rows(client, user_headers, store):
+    """Two columns for one field: keep the first, say so, keep importing."""
+    csv = (
+        "Brand,Product Name,Item Name,Final Price\n"
+        "Lion Dates,Lion Dates 450g,Ignore This One,185\n"
+    )
+
+    resp = _upload(client, user_headers, csv)
+
+    assert resp.status_code == 201, resp.text
+    body = resp.json()
+    assert body["added_count"] == 1
+    assert body["ignored_columns"] == [
+        {"column": "Item Name", "field": "product_name", "using_instead": "Product Name"}
+    ]
+    assert any("ignored" in w for w in body["warnings"])
+
+    stored = [p for _, products in store for p in products]
+    assert stored[0]["product_name"] == "Lion Dates 450g"
+
+
+def test_missing_required_columns_is_a_400_naming_the_headers(client, user_headers, store):
+    resp = _upload(client, user_headers, "Foo,Bar\n1,2\n")
+
+    assert resp.status_code == 400
+    detail = resp.json()["detail"]
+    assert "brand column" in detail
+    assert "Foo" in detail
+
+
+# ---------------------------------------------------------------------------
+# Row-level handling
+# ---------------------------------------------------------------------------
+def test_rows_missing_a_brand_are_errors_not_silent_skips(client, user_headers, store):
+    csv = (
+        "Brand,Product Name\n"
+        "Lion Dates,Lion Dates 450g\n"
+        ",Orphan Product\n"
+    )
+
+    resp = _upload(client, user_headers, csv)
+
+    body = resp.json()
+    assert resp.status_code == 201, resp.text
+    assert body["added_count"] == 1
+    assert body["error_count"] == 1
+    assert body["errors"][0]["row"] == 3
+    assert "brand" in body["errors"][0]["error"]
+
+
+def test_fully_blank_rows_are_skipped_and_counted(client, user_headers, store):
+    csv = (
+        "Brand,Product Name\n"
+        "Lion Dates,Lion Dates 450g\n"
+        ",\n"
+        ",\n"
+    )
+
+    resp = _upload(client, user_headers, csv)
+
+    body = resp.json()
+    assert body["added_count"] == 1
+    assert body["error_count"] == 0
+    assert body["skipped_blank_rows"] == 2
+
+
+def test_a_file_where_every_row_fails_is_never_a_success(client, user_headers, store):
+    csv = (
+        "Brand,Product Name\n"
+        ",Orphan One\n"
+        ",Orphan Two\n"
+    )
+
+    resp = _upload(client, user_headers, csv)
+
+    assert resp.status_code == 422, resp.text
+    assert "Nothing was saved" in resp.json()["detail"]
+    assert store == []
+
+
+def test_empty_and_headers_only_files_are_rejected(client, user_headers, store):
+    assert _upload(client, user_headers, b"").status_code == 400
+    assert _upload(client, user_headers, "Brand,Product Name\n").status_code == 400
+
+
+def test_unsupported_file_type_is_rejected(client, user_headers, store):
+    resp = _upload(client, user_headers, b"%PDF-1.4", name="products.pdf")
+
+    assert resp.status_code == 400
+    assert "Unsupported file type" in resp.json()["detail"]
+
+
+def test_oversized_row_count_is_rejected_before_any_write(client, user_headers, store):
+    rows = "".join(f"Lion Dates,Product {i}\n" for i in range(user_products.MAX_UPLOAD_ROWS + 1))
+
+    resp = _upload(client, user_headers, "Brand,Product Name\n" + rows)
+
+    assert resp.status_code == 413
+    assert store == []
+
+
+# ---------------------------------------------------------------------------
+# Batching: the collaborators must be called once per upload, not once per row
+# ---------------------------------------------------------------------------
+def test_one_embedding_call_and_one_write_per_brand(client, user_headers, monkeypatch, store):
+    embed_calls = []
+    sample_calls = []
+    # Pinned rather than inherited: the assertion below is about batching, and
+    # it should not silently pass because a .env happened to disable embeddings.
+    monkeypatch.setattr(user_products, "USE_EMBEDDINGS", True)
+    monkeypatch.setattr(user_products, "embed_texts",
+                        lambda texts: embed_calls.append(len(texts)) or [[0.0] * 384 for _ in texts])
+    monkeypatch.setattr(user_products, "get_products_by_brand",
+                        lambda brand, **k: sample_calls.append(brand) or [])
+
+    csv = "Brand,Product Name\n" + "".join(
+        f"Lion Dates,Lion Dates {i}g\n" for i in range(10)
+    )
+
+    resp = _upload(client, user_headers, csv)
+
+    assert resp.json()["added_count"] == 10
+    assert embed_calls == [10], "embeddings must be generated in one batched call"
+    assert len(store) == 1, "one write per brand, not one per row"
+    assert len(sample_calls) == 1, "one brand-sample read per brand, not one per row"
+
+
+def test_products_still_save_when_embeddings_are_disabled(client, user_headers, monkeypatch, store):
+    """USE_EMBEDDINGS=false must skip the model, not the row."""
+    def fail_if_called(texts):
+        raise AssertionError("embed_texts must not be called when USE_EMBEDDINGS is false")
+
+    monkeypatch.setattr(user_products, "USE_EMBEDDINGS", False)
+    monkeypatch.setattr(user_products, "embed_texts", fail_if_called)
+
+    resp = _upload(client, user_headers, TEMPLATE_CSV)
+
+    assert resp.status_code == 201, resp.text
+    assert resp.json()["added_count"] == 2
+    assert all(p["embedding"] is None for _, products in store for p in products)
+
+
+def test_a_failed_embedding_does_not_lose_the_product(client, user_headers, monkeypatch, store):
+    """Semantic search is a feature of the row; it is not the row."""
+    monkeypatch.setattr(user_products, "USE_EMBEDDINGS", True)
+    monkeypatch.setattr(user_products, "embed_texts",
+                        lambda texts: (_ for _ in ()).throw(RuntimeError("model not downloaded")))
+
+    resp = _upload(client, user_headers, TEMPLATE_CSV)
+
+    assert resp.status_code == 201, resp.text
+    assert resp.json()["added_count"] == 2
+
+
+def test_the_seed_catalog_is_only_written_for_rows_the_database_took(
+    client, user_headers, monkeypatch, store
+):
+    """Dual persistence must not become divergent persistence."""
+    catalog_writes = []
+    monkeypatch.setattr(user_products, "upsert_products_into_catalog_file",
+                        lambda brand, products: catalog_writes.append((brand, len(products))))
+
+    def unavailable(brand, products, cleanup=False):
+        raise VectorStoreUnavailable("database unreachable")
+
+    monkeypatch.setattr(user_products, "upsert_brand_products", unavailable)
+
+    resp = _upload(client, user_headers, TEMPLATE_CSV)
+
+    assert resp.status_code == 503
+    assert catalog_writes == [], "the JSON catalog must not gain products the database refused"