Skip to content

Commit 294b17e

Browse files
committed
fix(spreadsheet): reject duplicate headers in get_all_values path
Fail fast with GSpreadException when worksheet header row contains duplicates, use zip(strict=True) after row padding, and add regression tests for the helper and read_spreadsheet integration.
1 parent e62f92d commit 294b17e

2 files changed

Lines changed: 52 additions & 2 deletions

File tree

application/tests/spreadsheet_test.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,13 @@
44
from pprint import pprint
55
from unittest import mock
66

7+
import gspread
8+
79
from application import create_app, sqla # type: ignore
810
from application.database import db
911
from application.defs import cre_defs as defs
1012
from application.utils.spreadsheet import *
13+
from application.utils.spreadsheet import _records_from_worksheet_values
1114
from application.tests.utils.data_gen import export_format_data
1215

1316

@@ -216,3 +219,40 @@ def test_read_spreadsheet_short_row_padded(
216219
)
217220

218221
self.assertEqual(result["Short Row"], [{"Col A": "only-a", "Col B": ""}])
222+
223+
def test_records_from_worksheet_values_duplicate_headers(self) -> None:
224+
with self.assertRaises(gspread.exceptions.GSpreadException) as ctx:
225+
_records_from_worksheet_values(
226+
[["Col A", "Col B", "Col A"], ["x", "y", "z"]]
227+
)
228+
self.assertIn("Duplicate worksheet headers", str(ctx.exception))
229+
self.assertIn("Col A", str(ctx.exception))
230+
231+
@mock.patch("application.utils.spreadsheet.gspread.service_account")
232+
@mock.patch("application.utils.spreadsheet.gspread.oauth")
233+
def test_read_spreadsheet_duplicate_headers(
234+
self, mock_oauth, mock_service_account
235+
) -> None:
236+
fake_ws = mock.MagicMock()
237+
fake_ws.title = "Dup Headers"
238+
fake_ws.get_all_values.return_value = [
239+
["Name", "Name"],
240+
["row"],
241+
]
242+
243+
fake_sh = mock.MagicMock()
244+
fake_sh.worksheets.return_value = [fake_ws]
245+
246+
fake_client = mock.MagicMock()
247+
fake_client.open_by_url.return_value = fake_sh
248+
mock_oauth.return_value = fake_client
249+
mock_service_account.return_value = fake_client
250+
251+
result = read_spreadsheet(
252+
"https://example.com/fake-spreadsheet-url",
253+
"Test Spreadsheet",
254+
validate=False,
255+
parse_numbered_only=False,
256+
)
257+
258+
self.assertEqual(result, {})

application/utils/spreadsheet.py

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,24 @@ def findDups(x):
2727

2828

2929
def _records_from_worksheet_values(rows: List[List[Any]]) -> List[Dict[str, Any]]:
30-
"""Build row dicts from raw gspread values without numeric coercion."""
30+
"""Build row dicts from raw worksheet cell values without numeric coercion.
31+
32+
Uses the first row as column headers. Short data rows are padded with empty
33+
strings. Raises ``GSpreadException`` when duplicate header names are present
34+
so callers fail fast instead of silently collapsing columns in ``dict(zip)``.
35+
"""
3136
if not rows:
3237
return []
3338
headers = rows[0]
39+
duplicates = sorted(findDups(headers))
40+
if duplicates:
41+
raise gspread.exceptions.GSpreadException(
42+
f"Duplicate worksheet headers: {duplicates}"
43+
)
3444
records: List[Dict[str, Any]] = []
3545
for row in rows[1:]:
3646
padded = list(row) + [""] * max(0, len(headers) - len(row))
37-
records.append(dict(zip(headers, padded[: len(headers)])))
47+
records.append(dict(zip(headers, padded[: len(headers)], strict=True)))
3848
return records
3949

4050

0 commit comments

Comments
 (0)