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
|
|
@ -6,12 +6,12 @@ import json
|
|||
import mimetypes
|
||||
import re
|
||||
import secrets
|
||||
import time
|
||||
from datetime import timedelta
|
||||
from pathlib import Path
|
||||
from urllib.parse import urlencode
|
||||
|
||||
import markdown
|
||||
import openpyxl
|
||||
from django.conf import settings
|
||||
from django.contrib import messages
|
||||
from django.contrib.auth import views as auth_views
|
||||
|
|
@ -107,7 +107,7 @@ class ArtifactListCreateView(APIView):
|
|||
return HttpResponseForbidden("Invalid or missing API token")
|
||||
|
||||
if request.content_type and request.content_type.startswith("multipart/"):
|
||||
artifact = create_binary_artifact(request.data, user)
|
||||
artifact = create_binary_artifact(request.data, request, user)
|
||||
if isinstance(artifact, Response):
|
||||
return artifact
|
||||
apply_tag_colors(request.data)
|
||||
|
|
@ -616,7 +616,36 @@ def create_text_artifact(data, request, user):
|
|||
return artifact
|
||||
|
||||
|
||||
def create_binary_artifact(data, user):
|
||||
XLSX_CONTENT_TYPE = "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet"
|
||||
|
||||
|
||||
def is_xlsx_upload(uploaded, content_type):
|
||||
if content_type == XLSX_CONTENT_TYPE:
|
||||
return True
|
||||
return Path(uploaded.name).suffix.lower() == ".xlsx"
|
||||
|
||||
|
||||
def xlsx_to_csv_text(uploaded):
|
||||
"""Converts the active sheet of an uploaded .xlsx workbook to CSV text.
|
||||
|
||||
Only the active sheet is used - other sheets in a multi-sheet workbook
|
||||
are silently dropped. data_only=True reads cached formula *results*
|
||||
rather than formula source, since a results grid is what's useful here.
|
||||
"""
|
||||
workbook = openpyxl.load_workbook(io.BytesIO(uploaded.read()), data_only=True, read_only=True)
|
||||
sheet = workbook.active
|
||||
buffer = io.StringIO()
|
||||
# lineterminator="\n": csv.writer defaults to "\r\n", which then gets a
|
||||
# second newline translation from Path.write_text()'s own text-mode
|
||||
# handling when this string is saved to disk, producing a blank line
|
||||
# between every row on Windows.
|
||||
writer = csv.writer(buffer, lineterminator="\n")
|
||||
for row in sheet.iter_rows(values_only=True):
|
||||
writer.writerow(["" if cell is None else cell for cell in row])
|
||||
return buffer.getvalue()
|
||||
|
||||
|
||||
def create_binary_artifact(data, request, user):
|
||||
uploaded = data.get("file")
|
||||
required = ["source", "title"]
|
||||
missing = [field for field in required if not data.get(field)]
|
||||
|
|
@ -628,11 +657,36 @@ def create_binary_artifact(data, user):
|
|||
if source_error:
|
||||
return source_error
|
||||
|
||||
title = data["title"]
|
||||
content_type = getattr(uploaded, "content_type", "") or mimetypes.guess_type(uploaded.name)[0] or ""
|
||||
|
||||
if is_xlsx_upload(uploaded, content_type):
|
||||
try:
|
||||
content = xlsx_to_csv_text(uploaded)
|
||||
except Exception as exc:
|
||||
return Response({"detail": f"Could not read .xlsx file: {exc}"}, status=400)
|
||||
# Built as a plain dict via .get() rather than {**data, ...}: `data` is
|
||||
# a QueryDict for multipart uploads, and spreading a QueryDict with **
|
||||
# bypasses its __getitem__ override and exposes its internal
|
||||
# one-value-per-list storage directly, silently wrapping every value
|
||||
# (title included) in a list.
|
||||
text_data = {"source": data.get("source"), "title": data.get("title")}
|
||||
for optional_field in (
|
||||
"tags", "source_type", "source_name", "source_id", "source_prompt", "metadata",
|
||||
):
|
||||
# Only set if actually provided - create_text_artifact's own
|
||||
# data.get(field, default) calls need the key absent, not None,
|
||||
# to fall through to their intended defaults.
|
||||
value = data.get(optional_field)
|
||||
if value is not None:
|
||||
text_data[optional_field] = value
|
||||
text_data["kind"] = Artifact.CSV
|
||||
text_data["content"] = content
|
||||
return create_text_artifact(text_data, request, user)
|
||||
|
||||
now = timezone.localtime()
|
||||
artifact_id = generate_id()
|
||||
source = slugify(data["source"]) or "unknown"
|
||||
title = data["title"]
|
||||
content_type = getattr(uploaded, "content_type", "") or mimetypes.guess_type(uploaded.name)[0] or ""
|
||||
kind = binary_kind(content_type)
|
||||
|
||||
directory = Path(source) / now.date().isoformat()
|
||||
|
|
|
|||
26
docs/api.md
26
docs/api.md
|
|
@ -226,14 +226,38 @@ Required fields:
|
|||
| `title` | Required. |
|
||||
| `file` | Required file part. |
|
||||
|
||||
File kind is inferred from content type:
|
||||
File kind is inferred from content type (or file extension for `.xlsx`):
|
||||
|
||||
| Content type | Artifact kind |
|
||||
|---|---|
|
||||
| `image/*` | `image` |
|
||||
| `application/pdf` | `pdf` |
|
||||
| `.xlsx` (by content type or extension) | `csv` — converted server-side, see below |
|
||||
| Anything else | `binary` |
|
||||
|
||||
### `.xlsx` Upload → `csv` Kind
|
||||
|
||||
Uploading a `.xlsx` file doesn't create a `binary` artifact — it's read
|
||||
server-side (`openpyxl`) and converted into a `csv` artifact, so it renders
|
||||
in the same interactive grid view as a native CSV post. Known limitations:
|
||||
|
||||
- Only the workbook's **active sheet** is converted; other sheets in a
|
||||
multi-sheet workbook are silently dropped.
|
||||
- Formula cells use the **cached result** the spreadsheet application last
|
||||
calculated and saved, not a live re-evaluation — a workbook that's never
|
||||
actually been opened/calculated in real spreadsheet software (e.g. one
|
||||
built by a script that never triggered calculation) will show those cells
|
||||
blank.
|
||||
- Legacy `.xls` (pre-2007 binary format) is **not** supported, only modern
|
||||
`.xlsx`.
|
||||
- Rich spreadsheet formatting (bold, colors, merged cells, multi-level
|
||||
headers) is not preserved — like any `csv` artifact, it renders as a
|
||||
plain sortable/searchable table. If preserving that formatting matters
|
||||
more than interactivity, post as `html_fragment` with a hand-built
|
||||
`<table>` instead.
|
||||
|
||||
A corrupt or unreadable `.xlsx` file returns `400`.
|
||||
|
||||
PowerShell/curl example:
|
||||
|
||||
```powershell
|
||||
|
|
|
|||
|
|
@ -2,5 +2,6 @@ Django>=5.0,<6.0
|
|||
djangorestframework>=3.15,<4.0
|
||||
Markdown>=3.6,<4.0
|
||||
Pygments>=2.17,<3.0
|
||||
openpyxl>=3.1,<4.0
|
||||
gunicorn>=23.0,<24.0
|
||||
whitenoise>=6.7,<7.0
|
||||
|
|
|
|||
|
|
@ -120,6 +120,19 @@ python skills/daybook-posting/scripts/daybook_post.py results.csv
|
|||
python skills/daybook-posting/scripts/daybook_post.py --kind json --title "Run Summary" --content-file summary.json
|
||||
```
|
||||
|
||||
`.xlsx` spreadsheets work too, with no special flag needed — just `--file`
|
||||
(or post it positionally, same as any other file):
|
||||
|
||||
```bash
|
||||
python skills/daybook-posting/scripts/daybook_post.py balance_sheet.xlsx
|
||||
```
|
||||
|
||||
The server converts it to a `csv` artifact automatically (active sheet
|
||||
only; see `docs/api.md` for the full list of conversion limitations —
|
||||
notably, formulas need to have been calculated by real spreadsheet software
|
||||
that saved the file, and rich formatting like bold/colors/merged cells
|
||||
isn't preserved).
|
||||
|
||||
For other code that should render today, use `--file path --as-markdown` to
|
||||
post it as a Markdown fenced code block. Without `--as-markdown`, it uploads
|
||||
as a downloadable binary file.
|
||||
|
|
|
|||
|
|
@ -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