Convert uploaded .xlsx files to csv artifacts server-side
Uploading a .xlsx file no longer becomes a generic downloadable binary -
it's read server-side with openpyxl and converted to a csv artifact,
rendering in the same interactive grid as a native CSV post. Detected by
content type or .xlsx extension (some clients send generic/incorrect
content types for multipart uploads). Only the active sheet converts;
formula cells use whatever cached result the spreadsheet software last
saved (openpyxl doesn't evaluate formulas itself). Corrupt/unreadable
files return 400.
Since the existing posting script already forwards any non-text file via
multipart with no special-casing, `daybook_post.py balance_sheet.xlsx`
already works with zero script changes - verified live.
Two real bugs found and fixed while building this, both now covered by
regression tests:
- {**a_querydict, ...} silently wrapped every value (including title) in
a list - Python's ** spread on a dict subclass bypasses QueryDict's
__getitem__ override and exposes its internal one-list-per-key storage
directly. Fixed by building the delegated dict explicitly via .get().
- Pre-setting None for absent optional fields (e.g. source_prompt) defeated
create_text_artifact's own data.get(field, default) calls, since .get()
only falls through to its default when the key is *missing*, not when
the value is None - caused a NOT NULL constraint failure. Fixed by only
setting keys that were actually provided.
- csv.writer's default "\r\n" terminator was getting doubled by
Path.write_text()'s own newline translation on Windows, producing a
blank line between every row. Fixed with lineterminator="\n".
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
4145c37dfe
commit
4b5ec09ae2
6 changed files with 232 additions and 6 deletions
|
|
@ -1,8 +1,10 @@
|
|||
import json
|
||||
import shutil
|
||||
import tempfile
|
||||
from io import BytesIO
|
||||
from pathlib import Path
|
||||
|
||||
import openpyxl
|
||||
from django.contrib.auth import get_user_model
|
||||
from django.test import override_settings
|
||||
from django.urls import reverse
|
||||
|
|
@ -455,3 +457,75 @@ class ArtifactAPITestCase(APITestCase):
|
|||
|
||||
artifact = Artifact.objects.get(pk=artifact_id)
|
||||
self.assertEqual(artifact.source_name, "test-agent") # unchanged, not mangled
|
||||
|
||||
# -- xlsx upload converts to csv kind ------------------------------------
|
||||
|
||||
def make_xlsx(self, rows, filename="sheet.xlsx"):
|
||||
workbook = openpyxl.Workbook()
|
||||
sheet = workbook.active
|
||||
for row in rows:
|
||||
sheet.append(row)
|
||||
buffer = BytesIO()
|
||||
workbook.save(buffer)
|
||||
buffer.seek(0)
|
||||
buffer.name = filename
|
||||
return buffer
|
||||
|
||||
def test_xlsx_upload_converts_to_csv_kind(self):
|
||||
xlsx = self.make_xlsx([["Item", "Q1", "Q2"], ["Cash", 1000, 1200]])
|
||||
response = self.client.post(
|
||||
reverse("artifact-api"),
|
||||
{"source": "test-agent", "title": "Balance Sheet", "file": xlsx},
|
||||
format="multipart",
|
||||
**self.auth_headers(self.token),
|
||||
)
|
||||
self.assertEqual(response.status_code, 201)
|
||||
self.assertEqual(response.data["kind"], "csv")
|
||||
|
||||
artifact = Artifact.objects.get(pk=response.data["id"])
|
||||
raw_path = (self.artifacts_root / artifact.path).with_suffix(".csv")
|
||||
# read_text() normalizes line endings, so this is OS-portable rather
|
||||
# than asserting the exact on-disk bytes (which are "\r\n" on Windows).
|
||||
self.assertEqual(raw_path.read_text(encoding="utf-8"), "Item,Q1,Q2\nCash,1000,1200\n")
|
||||
|
||||
stored_html = (self.artifacts_root / artifact.path).read_text(encoding="utf-8")
|
||||
self.assertIn('"columns": ["Item", "Q1", "Q2"]', stored_html)
|
||||
self.assertIn('["Cash", "1000", "1200"]', stored_html)
|
||||
|
||||
def test_xlsx_upload_does_not_mangle_fields(self):
|
||||
"""Regression test: {**a_querydict} previously wrapped every value
|
||||
(including title) in a list - QueryDict's __getitem__ override is
|
||||
bypassed by the ** spread, exposing its internal list-per-key
|
||||
storage directly. Separately, pre-setting None for absent optional
|
||||
fields defeated create_text_artifact's own .get(field, default)
|
||||
calls (.get only falls through when the key is missing, not None),
|
||||
which previously caused a NOT NULL constraint failure on
|
||||
source_prompt."""
|
||||
xlsx = self.make_xlsx([["a", "b"]])
|
||||
response = self.client.post(
|
||||
reverse("artifact-api"),
|
||||
{
|
||||
"source": "test-agent",
|
||||
"title": "Untitled Sheet",
|
||||
"tags": "finance,q2",
|
||||
"file": xlsx,
|
||||
},
|
||||
format="multipart",
|
||||
**self.auth_headers(self.token),
|
||||
)
|
||||
self.assertEqual(response.status_code, 201)
|
||||
self.assertEqual(response.data["title"], "Untitled Sheet") # not ["Untitled Sheet"]
|
||||
self.assertEqual(response.data["source_prompt"], "") # defaulted, not None
|
||||
self.assertEqual([t["name"] for t in response.data["tags"]], ["finance", "q2"])
|
||||
|
||||
def test_xlsx_upload_rejects_corrupt_file(self):
|
||||
buffer = BytesIO(b"not a real xlsx file")
|
||||
buffer.name = "broken.xlsx"
|
||||
response = self.client.post(
|
||||
reverse("artifact-api"),
|
||||
{"source": "test-agent", "title": "Broken", "file": buffer},
|
||||
format="multipart",
|
||||
**self.auth_headers(self.token),
|
||||
)
|
||||
self.assertEqual(response.status_code, 400)
|
||||
|
||||
|
|
|
|||
60
tests/unit/test_xlsx_conversion.py
Normal file
60
tests/unit/test_xlsx_conversion.py
Normal file
|
|
@ -0,0 +1,60 @@
|
|||
from io import BytesIO
|
||||
from types import SimpleNamespace
|
||||
|
||||
import openpyxl
|
||||
from django.test import SimpleTestCase
|
||||
|
||||
from artifacts.views import XLSX_CONTENT_TYPE, is_xlsx_upload, xlsx_to_csv_text
|
||||
|
||||
|
||||
class IsXlsxUploadTestCase(SimpleTestCase):
|
||||
def test_matches_exact_content_type(self):
|
||||
upload = SimpleNamespace(name="whatever.bin")
|
||||
self.assertTrue(is_xlsx_upload(upload, XLSX_CONTENT_TYPE))
|
||||
|
||||
def test_matches_by_extension_regardless_of_content_type(self):
|
||||
upload = SimpleNamespace(name="sheet.xlsx")
|
||||
self.assertTrue(is_xlsx_upload(upload, "application/octet-stream"))
|
||||
self.assertTrue(is_xlsx_upload(upload, ""))
|
||||
|
||||
def test_extension_match_is_case_insensitive(self):
|
||||
upload = SimpleNamespace(name="SHEET.XLSX")
|
||||
self.assertTrue(is_xlsx_upload(upload, ""))
|
||||
|
||||
def test_rejects_other_files(self):
|
||||
upload = SimpleNamespace(name="report.pdf")
|
||||
self.assertFalse(is_xlsx_upload(upload, "application/pdf"))
|
||||
|
||||
|
||||
class XlsxToCsvTextTestCase(SimpleTestCase):
|
||||
def make_xlsx(self, rows):
|
||||
workbook = openpyxl.Workbook()
|
||||
sheet = workbook.active
|
||||
for row in rows:
|
||||
sheet.append(row)
|
||||
buffer = BytesIO()
|
||||
workbook.save(buffer)
|
||||
buffer.seek(0)
|
||||
return buffer
|
||||
|
||||
def test_converts_active_sheet_to_csv(self):
|
||||
upload = self.make_xlsx([["Item", "Q1", "Q2"], ["Cash", 1000, 1200]])
|
||||
content = xlsx_to_csv_text(upload)
|
||||
self.assertEqual(content, "Item,Q1,Q2\nCash,1000,1200\n")
|
||||
|
||||
def test_uses_single_line_terminator_not_doubled(self):
|
||||
"""Regression test: csv.writer's default "\\r\\n" gets a second
|
||||
newline translation from Path.write_text()'s text-mode handling when
|
||||
this string is later saved to disk, producing a blank line between
|
||||
every row on Windows - lineterminator="\\n" avoids embedding "\\r\\n"
|
||||
in the string in the first place, leaving the OS-level text-mode
|
||||
write as the only place "\\n" becomes "\\r\\n"."""
|
||||
upload = self.make_xlsx([["a"], ["b"], ["c"]])
|
||||
content = xlsx_to_csv_text(upload)
|
||||
self.assertNotIn("\r", content)
|
||||
self.assertEqual(content.count("\n"), 3)
|
||||
|
||||
def test_empty_cells_become_empty_strings(self):
|
||||
upload = self.make_xlsx([["a", "b"], ["x", None]])
|
||||
content = xlsx_to_csv_text(upload)
|
||||
self.assertEqual(content, "a,b\nx,\n")
|
||||
Loading…
Add table
Add a link
Reference in a new issue