Skip to content

fix: allow null position in Mapping for disabled mappings#126

Merged
Subterrane merged 2 commits into
mainfrom
copilot/fix-update-mapping-status-disabled
May 12, 2026
Merged

fix: allow null position in Mapping for disabled mappings#126
Subterrane merged 2 commits into
mainfrom
copilot/fix-update-mapping-status-disabled

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 12, 2026

The OneLogin API returns null for position on disabled mappings, but position was typed as required StrictInt, causing a pydantic ValidationError on deserialization. Passing the existing position value as a workaround then triggered a 422 from the API ("Position cannot be set while the mapping is not enabled").

Changes

  • onelogin/models/mapping.py: Relax position from StrictInt = Field(...) to Optional[StrictInt] = Field(None, ...). The existing exclude_none=True in to_dict() already omits None positions from serialized request bodies, so disabled-mapping updates no longer trigger the 422.

  • test/test_mapping.py: Add tests covering position=None round-trip: deserialization from an API response with "position": null, deserialization when position is absent, and confirmation that to_dict() excludes a None position.

Example

# Previously raised pydantic ValidationError
mapping = Mapping.from_dict({
    "id": 42,
    "name": "My Mapping",
    "enabled": False,   # disabled → API returns null for position
    "match": "all",
    "position": None,   # ← was rejected by StrictInt
    "conditions": [...],
    "actions": [...],
})

# to_dict() correctly omits position=None from the update body
assert "position" not in mapping.to_dict()

The OneLogin API returns null for position when a mapping is disabled.
StrictInt rejected None, causing a pydantic ValidationError on
deserialization. Callers then tried using the existing position value
in update requests, which the API rejected with 422 "Position cannot
be set while the mapping is not enabled."

Change position to Optional[StrictInt] = None. The existing
exclude_none=True in to_dict() already omits None positions from
update request bodies, so no additional change is needed there.

Agent-Logs-Url: https://github.com/onelogin/onelogin-python-sdk/sessions/f6b0e517-ee3b-4af3-93a5-c112abaf2a73

Co-authored-by: Subterrane <5290140+Subterrane@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix validation error when updating mapping status to disabled fix: allow null position in Mapping for disabled mappings May 12, 2026
Copilot AI requested a review from Subterrane May 12, 2026 21:10
@Subterrane Subterrane marked this pull request as ready for review May 12, 2026 21:33
@Subterrane Subterrane merged commit ce40412 into main May 12, 2026
7 checks passed
Subterrane added a commit that referenced this pull request May 12, 2026
The vast majority of test/ files were generator-stub artifacts: they
defined a single `testFoo` method whose body was just commented-out
example code inside a triple-quoted string. They contained zero
assertions, so pytest discovered and "passed" them without exercising
any code. This made the suite report "358 passed" while real coverage
came from only 8 files (66 actual test methods).

The fake green made CI a dangerous signal — a regression that broke
any of the 178 stubbed-out models would still "pass" because the
corresponding test asserted nothing. The bug fix in PR #126
(`AB#none`: position nullability) was only caught because
test_mapping.py happened to be one of the 8 real test files.

This commit deletes the 178 stubs and keeps the 8 real test files:

  test/test_create_role201_response_inner.py
  test/test_get_auth_factors_fix.py
  test/test_mapping.py
  test/test_roles_api.py
  test/test_saml2_import_compatibility.py
  test/test_user_custom_attributes.py
  test/test_user_manager_user_id_fix.py
  test/test_user_mappings_api.py

Post-deletion: `pytest test/` reports 66 passed, which is the honest
real-test number. Future bug fixes should bring their own real tests
in the style of test_mapping.py::test_from_dict_with_null_position
(the test we added in 3.2.8).
Subterrane added a commit that referenced this pull request May 12, 2026
Auth servers in OneLogin are implemented as Apps rows with auth_type=9.
The underlying apps.description column is nullable (Rails default —
no NOT NULL constraint in core-api/db/schema.rb), so:

  GET /api/2/api_authorizations/{id}

returns `"description": null` for any auth server created without a
description. The SDK had typed AuthServer.description as required
StrictStr, causing pydantic ValidationError on those responses.

Same fix shape as PR #126 (Mapping.position): relax to
Optional[StrictStr], defaulting to None. The existing
exclude_none=True in to_dict() already drops it from request bodies
when unset, so no other change is needed.

Also adds test/test_auth_server.py with the standard four-case
nullability pattern (with-value, with-null, with-absent, exclude-on-serialize),
matching test_mapping.py::test_from_dict_with_null_position.
@Subterrane Subterrane deleted the copilot/fix-update-mapping-status-disabled branch May 12, 2026 23:05
Subterrane added a commit that referenced this pull request May 12, 2026
Two concrete fixes plus general tightening:

1. The Getting Started example created Configuration twice — once with
   the host URL, then immediately reassigned the variable with
   username/password (dropping the host). A user copy-pasting the
   example would silently hit the OneLogin default host instead of
   their own subdomain. Collapsed into a single Configuration() call
   that accepts host + credentials in one shot.

2. The Release Process section said the publish workflow "updates
   `version` in pyproject.toml and `__version__` in onelogin/__init__.py".
   That's technically true but misleading — the workflow runs `sed -i`
   inside the ephemeral CI runner and never commits back to main. The
   source tree drift this caused was exactly what PR #128 fixed
   manually. The section now states the truth plainly: edits happen in
   the runner only, the source tree may lag, reconcile periodically.

Also:

- Trimmed redundant "First, install the package" sections (Tests and
  Installation said the same thing twice).
- Dropped per-dependency version pin list in Requirements — that
  belongs in `pyproject.toml`, not in two places.
- Removed inline auto-generated-comment noise from the example
  (`# GenerateTokenRequest | Request Body to ...`) and the redundant
  explicit `content_type="application/json"` which is the default.
- Added a one-line pointer to `docs/` for per-endpoint documentation.
- Added `[lint]` to the dev install instructions (introduced in PR #130).
- Aligned the example tag format with current convention (`3.2.9` not
  `v3.2.9`; only `v3.2.0` is the legacy outlier).
- Added a ValidationError-on-response troubleshooting entry that
  points users at PR #126 / #131 as worked examples — they're the
  most common bug class with this SDK and a quick issue gets fixed
  fast.

Line count: 163 → 85.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update mapping to status = disabled fails validation

2 participants