Escape quotes in URL parameters by jeffandms · Pull Request #10 · microsoft/PowerPlatform-DataverseClient-Python

jeffandms · 2025-09-16T13:25:36Z

Scenarios like FIND where the URL includes record values need to escape quotation marks, otherwise an invalid URL is generated.

- Document type aliases in tables.create() columns param (#4) - Clarify SQL description in client namespace summary (#8) - Add :raises: for HttpError, ValidationError, ValueError across all operation methods (#6) - Fix query.sql() to document ValidationError instead of incorrect SQLParseError - Restore post-call validation in records.create() matching original client.create() behavior (#9) - Improve deprecated get() docstring with explicit guidance on when to use records.get() vs query.get() (#10) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

@overload

#102) * Add operation namespaces (client.records, client.query, client.tables) Reorganize SDK public API into three operation namespaces per the SDK Redesign Summary. New namespace methods use cleaner signatures (keyword-only params, @overload for single/bulk, renamed table params) while all existing flat methods (client.create, client.get, etc.) continue to work unchanged with deprecation warnings that point to the new equivalents. Key changes: - records.create() returns str for single, list[str] for bulk - query.get() handles paginated multi-record queries - tables.create() uses solution= and primary_column= keyword args - 40 new unit tests (75 total, all passing) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix PR review comments: docstrings, error docs, and create() validation - Document type aliases in tables.create() columns param (#4) - Clarify SQL description in client namespace summary (#8) - Add :raises: for HttpError, ValidationError, ValueError across all operation methods (#6) - Fix query.sql() to document ValidationError instead of incorrect SQLParseError - Restore post-call validation in records.create() matching original client.create() behavior (#9) - Improve deprecated get() docstring with explicit guidance on when to use records.get() vs query.get() (#10) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Use :class: role in :type:/:rtype: directives for Sphinx cross-references Aligns with existing codebase convention and ensures clickable hyperlinks in generated API documentation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: tpellissier <tpellissier@microsoft.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…_scalar` (#146) Adds test coverage gaps identified in the PR #98 review: direct tests for `_normalize_scalar()` and an end-to-end mocked CRUD flow for `DataFrameOperations`. ## `tests/unit/test_pandas_helpers.py` - New `TestNormalizeScalar` class (9 tests) directly exercising `_normalize_scalar()`: - NumPy types (`np.integer`, `np.floating`, `np.bool_`) → Python natives - `pd.Timestamp` → ISO 8601 string - Native Python types and `None` pass through unchanged ## `tests/unit/test_dataframe_operations.py` - New `TestDataFrameEndToEnd` class (2 tests): - Full mocked CRUD cycle: `create → get → update → delete` - Verifies NumPy types are normalized to Python-native values before reaching the API layer ## Notes - `filter` parameter kept as-is (consistent with `records.get()` API; repo convention prohibits `# noqa` suppression) - `DataFrameOperations` not re-exported from top-level `__init__.py` (repo convention: package `__init__.py` files use `__all__ = []`)  <details> <summary>Original prompt</summary> ## Context This PR addresses the remaining unresolved review comments from PR #98 (#98) and adds comprehensive unit tests for the DataFrame operations. The PR #98 adds DataFrame CRUD wrappers (`client.dataframe.get()`, `client.dataframe.create()`, `client.dataframe.update()`, `client.dataframe.delete()`) to the Dataverse Python SDK. The author has addressed many review comments but several remain unresolved. ## Current State of the Code The branch `users/zhaodongwang/dataFrameExtensionClaude` has the latest code. Key files: ### `src/PowerPlatform/Dataverse/utils/_pandas.py` (current) ```python # Copyright (c) Microsoft Corporation. # Licensed under the MIT license. """Internal pandas helpers""" from __future__ import annotations from typing import Any, Dict, List import numpy as np import pandas as pd def _normalize_scalar(v: Any) -> Any: """Convert numpy scalar types to their Python native equivalents.""" if isinstance(v, pd.Timestamp): return v.isoformat() if isinstance(v, np.integer): return int(v) if isinstance(v, np.floating): return float(v) if isinstance(v, np.bool_): return bool(v) return v def dataframe_to_records(df: pd.DataFrame, na_as_null: bool = False) -> List[Dict[str, Any]]: """Convert a DataFrame to a list of dicts, normalizing values for JSON serialization.""" records = [] for row in df.to_dict(orient="records"): clean = {} for k, v in row.items(): if pd.api.types.is_scalar(v): if pd.notna(v): clean[k] = _normalize_scalar(v) elif na_as_null: clean[k] = None else: clean[k] = v records.append(clean) return records ``` ### `src/PowerPlatform/Dataverse/operations/dataframe.py` (current - 305 lines) The `DataFrameOperations` class provides get/create/update/delete methods. Key points: - `get()` returns a single consolidated DataFrame (iterates all pages internally) - `create()` validates non-empty, validates ID count matches - `update()` validates id_column exists, validates IDs are non-empty strings, validates at least one change column exists; has `clear_nulls` parameter - `delete()` validates ids is Series, validates IDs are non-empty strings, special-cases single ID ### `src/PowerPlatform/Dataverse/operations/__init__.py` (current) ```python from .dataframe import DataFrameOperations __all__ = ["DataFrameOperations"] ``` ### `src/PowerPlatform/Dataverse/__init__.py` (current) ```python from importlib.metadata import version __version__ = version("PowerPlatform-Dataverse-Client") __all__ = ["__version__"] ``` ### `src/PowerPlatform/Dataverse/client.py` (current) Already imports and exposes `DataFrameOperations` as `self.dataframe`. ## Issues to Fix ### 1. `filter` parameter shadows Python built-in (item #8) In `dataframe.py` `get()` method, the parameter `filter` shadows the Python built-in `filter()`. Since this mirrors the existing `records.get()` API which also uses `filter`, renaming is risky for API consistency. The safe fix is to add a `# noqa: A002` comment on the parameter and leave it as-is for API consistency (the base `records.get()` already uses `filter`). Alternatively, rename to `filter_expr` with an alias for backward compatibility. **Decision: keep `filter` for API consistency with existing `records.get()`, but suppress the lint warning.** ### 2. Missing `__init__.py` export for `DataFrameOperations` (item #9) The `operations/__init__.py` already exports `DataFrameOperations`. However, the top-level `src/PowerPlatform/Dataverse/__init__.py` does NOT export it. Add the export there so users can do `from PowerPlatform.Dataverse import DataFrameOperations` if needed. ### 3. Comprehensive unit tests (item #10) The existing `tests/unit/test_client_dataframe.py` has 365 lines of tests. We need to add MORE tests to ensure full coverage. Specifically add tests for: **Unit tests for `_pandas.py` helpers:** - `_normalize_scalar` with np.int64, np.float64, np.bool_, pd.Timestamp, regular Python types - `dataframe_to_records` with NaN handling (na_as_null=True vs False) - `dataframe_to_records` with Timestamp conversion - `dataframe_to_records` with non-scalar values (lists, dicts in cells) - `dataframe_to_records` with numpy scalar types in DataFrame - `dataframe_to_records` with empty DataFrame - `dataframe_to_records` with mixed types **Unit tests for `DataFrameOperations`:** - `get()` single record - `get()` multi-page results concatenated - `get()` empty results - `get()` with all parameters passed through - `create()` with valid DataFrame - `create()` with empty DataFrame (should raise ValueError) - `create()` with non-DataFrame input (should raise TypeError) - `create()` ID count mismatch (should raise ValueError) - `update()` with valid DataFrame - `update()` single record path - `... </details>  *This pull request was created from Copilot chat.* >  --- 🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. [Learn more about Advanced Security.](https://gh.io/cca-advanced-security) --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: saurabhrb <32964911+saurabhrb@users.noreply.github.com>

Escape quotes in URL parameters

2a9b009

tpellissier-msft approved these changes Sep 16, 2025

View reviewed changes

tpellissier-msft merged commit e93fa6e into main Sep 16, 2025
3 checks passed

zhaodongwang-msft deleted the user/jeffand/quote branch February 13, 2026 21:52

Copilot AI mentioned this pull request Mar 17, 2026

Add comprehensive unit tests for DataFrame operations and _normalize_scalar #146

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Escape quotes in URL parameters#10

Escape quotes in URL parameters#10
tpellissier-msft merged 1 commit intomainfrom
user/jeffand/quote

jeffandms commented Sep 16, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

jeffandms commented Sep 16, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants