Skip to content

Commit 50d8038

Browse files
tpellissierclaude
andcommitted
Fix Copilot review round 3: reject bare str for select param, update docstrings
- Add TypeError guard rejecting bare str passed to select parameter (prevents character-joined $select like "L,o,g,i,c,a,l,N,a,m,e") - Update docstrings in _odata.py and tables.py to document that select=[] is equivalent to None (all properties returned) - Add test_select_bare_string_raises_type_error unit test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 448052c commit 50d8038

File tree

3 files changed

+17
-3
lines changed

3 files changed

+17
-3
lines changed

src/PowerPlatform/Dataverse/data/_odata.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1440,8 +1440,9 @@ def _list_tables(
14401440
:param select: Optional list of property names to project via
14411441
``$select``. Values are passed as-is (PascalCase) because
14421442
``EntityDefinitions`` uses PascalCase property names.
1443-
When ``None`` (the default), no ``$select`` is applied and all
1444-
properties are returned.
1443+
When ``None`` (the default) or an empty list, no ``$select`` is
1444+
applied and all properties are returned. Passing a bare string
1445+
raises ``TypeError``.
14451446
:type select: ``list[str]`` or ``None``
14461447
14471448
:return: Metadata entries for non-private tables (may be empty).
@@ -1456,6 +1457,11 @@ def _list_tables(
14561457
else:
14571458
combined_filter = base_filter
14581459
params: Dict[str, str] = {"$filter": combined_filter}
1460+
if select is not None and isinstance(select, str):
1461+
raise TypeError(
1462+
"select must be a list of strings, not a single str; "
1463+
"did you mean ['" + select + "'] instead of '" + select + "'?"
1464+
)
14591465
if select:
14601466
params["$select"] = ",".join(select)
14611467
r = self._request("get", url, params=params)

src/PowerPlatform/Dataverse/operations/tables.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,8 @@ def list(
203203
Property names must use the exact PascalCase names from the
204204
``EntityDefinitions`` metadata (e.g.
205205
``["LogicalName", "SchemaName", "DisplayName"]``).
206-
When ``None`` (the default), all properties are returned.
206+
When ``None`` (the default) or an empty list, all properties are
207+
returned.
207208
:type select: :class:`list` of :class:`str` or None
208209
209210
:return: List of EntityDefinition metadata dictionaries.

tests/unit/data/test_odata_internal.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,13 @@ def test_select_single_property(self):
247247
params = call_kwargs.kwargs.get("params") or call_kwargs[1].get("params", {})
248248
self.assertEqual(params["$select"], "LogicalName")
249249

250+
def test_select_bare_string_raises_type_error(self):
251+
"""_list_tables(select='LogicalName') raises TypeError for bare str."""
252+
self._setup_response([])
253+
with self.assertRaises(TypeError) as ctx:
254+
self.od._list_tables(select="LogicalName")
255+
self.assertIn("list of strings", str(ctx.exception))
256+
250257

251258
class TestUpsert(unittest.TestCase):
252259
"""Unit tests for _ODataClient._upsert."""

0 commit comments

Comments
 (0)