Skip to content

Commit c42dcae

Browse files
committed
Allow for same param name but different contextual usage
The param provided by users is `user_id`/ `group_id` / `directory_id`; this is expected as `user` / `group` / `directory` by the API. BUT, paginating through these endpoints requires the `*_id` version of these params, so we need to add some kind of translation to change the value, depending on context
1 parent 082178f commit c42dcae

2 files changed

Lines changed: 81 additions & 10 deletions

File tree

src/workos/directory_sync.py

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
from typing import Optional, Protocol
1+
from typing import Any, Dict, Optional, Protocol
22

33
from workos.types.directory_sync.list_filters import (
44
DirectoryGroupListFilters,
@@ -32,6 +32,24 @@
3232
Directory, DirectoryListFilters, ListMetadata
3333
]
3434

35+
# Mapping from SDK parameter names to API parameter names
36+
PARAM_KEY_MAPPING = {
37+
"directory_id": "directory",
38+
"group_id": "group",
39+
"user_id": "user",
40+
}
41+
42+
43+
def _prepare_request_params(
44+
list_params: DirectoryUserListFilters | DirectoryGroupListFilters,
45+
) -> Dict[str, Any]:
46+
"""Convert list_params to API request params by renaming keys."""
47+
request_params: Dict[str, Any] = dict(list_params)
48+
for sdk_key, api_key in PARAM_KEY_MAPPING.items():
49+
if sdk_key in request_params:
50+
request_params[api_key] = request_params.pop(sdk_key)
51+
return request_params
52+
3553

3654
class DirectorySyncModule(Protocol):
3755
"""Offers methods through the WorkOS Directory Sync service."""
@@ -191,7 +209,7 @@ def list_users(
191209
response = self._http_client.request(
192210
"directory_users",
193211
method=REQUEST_METHOD_GET,
194-
params=list_params,
212+
params=_prepare_request_params(list_params),
195213
)
196214

197215
return WorkOSListResource(
@@ -225,7 +243,7 @@ def list_groups(
225243
response = self._http_client.request(
226244
"directory_groups",
227245
method=REQUEST_METHOD_GET,
228-
params=list_params,
246+
params=_prepare_request_params(list_params),
229247
)
230248

231249
return WorkOSListResource[
@@ -329,7 +347,7 @@ async def list_users(
329347
response = await self._http_client.request(
330348
"directory_users",
331349
method=REQUEST_METHOD_GET,
332-
params=list_params,
350+
params=_prepare_request_params(list_params),
333351
)
334352

335353
return WorkOSListResource(
@@ -354,6 +372,7 @@ async def list_groups(
354372
"after": after,
355373
"order": order,
356374
}
375+
357376
if user_id is not None:
358377
list_params["user_id"] = user_id
359378
if directory_id is not None:
@@ -362,7 +381,7 @@ async def list_groups(
362381
response = await self._http_client.request(
363382
"directory_groups",
364383
method=REQUEST_METHOD_GET,
365-
params=list_params,
384+
params=_prepare_request_params(list_params),
366385
)
367386

368387
return WorkOSListResource[

tests/test_directory_sync.py

Lines changed: 57 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
from typing import Union
22

33
import pytest
4-
from workos.directory_sync import AsyncDirectorySync, DirectorySync
4+
5+
from workos.directory_sync import _prepare_request_params
56

67
from tests.types.test_auto_pagination_function import TestAutoPaginationFunction
78
from tests.utils.fixtures.mock_directory import (
@@ -13,6 +14,7 @@
1314
from tests.utils.fixtures.mock_directory_user import MockDirectoryUser
1415
from tests.utils.list_resource import list_data_to_dicts, list_response_of
1516
from tests.utils.syncify import syncify
17+
from workos.directory_sync import AsyncDirectorySync, DirectorySync
1618

1719

1820
def api_directory_to_sdk(directory):
@@ -145,7 +147,7 @@ def test_list_users_with_directory(
145147
assert request_kwargs["url"].endswith("/directory_users")
146148
assert request_kwargs["method"] == "get"
147149
assert request_kwargs["params"] == {
148-
"directory_id": "directory_id",
150+
"directory": "directory_id",
149151
"limit": 10,
150152
"order": "desc",
151153
}
@@ -163,7 +165,7 @@ def test_list_users_with_group(
163165
assert request_kwargs["url"].endswith("/directory_users")
164166
assert request_kwargs["method"] == "get"
165167
assert request_kwargs["params"] == {
166-
"group_id": "directory_grp_id",
168+
"group": "directory_grp_id",
167169
"limit": 10,
168170
"order": "desc",
169171
}
@@ -181,7 +183,7 @@ def test_list_groups_with_directory(
181183
assert request_kwargs["url"].endswith("/directory_groups")
182184
assert request_kwargs["method"] == "get"
183185
assert request_kwargs["params"] == {
184-
"directory_id": "directory_id",
186+
"directory": "directory_id",
185187
"limit": 10,
186188
"order": "desc",
187189
}
@@ -199,7 +201,7 @@ def test_list_groups_with_user(
199201
assert request_kwargs["url"].endswith("/directory_groups")
200202
assert request_kwargs["method"] == "get"
201203
assert request_kwargs["params"] == {
202-
"user_id": "directory_user_id",
204+
"user": "directory_user_id",
203205
"limit": 10,
204206
"order": "desc",
205207
}
@@ -371,3 +373,53 @@ def test_directory_user_groups_auto_pagination(
371373
list_function=self.directory_sync.list_groups,
372374
expected_all_page_data=mock_directory_groups_multiple_data_pages,
373375
)
376+
377+
378+
class TestPrepareRequestParams:
379+
"""Tests for SDK-to-API parameter name translation.
380+
381+
The SDK uses Pythonic parameter names (directory_id, group_id, user_id)
382+
but the WorkOS API expects shorter names (directory, group, user).
383+
The _prepare_request_params function handles this translation.
384+
385+
See: https://github.com/workos/workos-python/issues/511
386+
See: https://github.com/workos/workos-python/issues/519
387+
"""
388+
389+
def test_translates_directory_id_to_directory(self):
390+
params = {"directory_id": "dir_123", "limit": 10}
391+
result = _prepare_request_params(params)
392+
assert "directory" in result
393+
assert "directory_id" not in result
394+
assert result["directory"] == "dir_123"
395+
396+
def test_translates_group_id_to_group(self):
397+
params = {"group_id": "grp_123", "limit": 10}
398+
result = _prepare_request_params(params)
399+
assert "group" in result
400+
assert "group_id" not in result
401+
assert result["group"] == "grp_123"
402+
403+
def test_translates_user_id_to_user(self):
404+
params = {"user_id": "usr_123", "limit": 10}
405+
result = _prepare_request_params(params)
406+
assert "user" in result
407+
assert "user_id" not in result
408+
assert result["user"] == "usr_123"
409+
410+
def test_preserves_non_id_params(self):
411+
params = {
412+
"directory_id": "dir_123",
413+
"limit": 10,
414+
"order": "desc",
415+
"after": "cursor",
416+
}
417+
result = _prepare_request_params(params)
418+
assert result["limit"] == 10
419+
assert result["order"] == "desc"
420+
assert result["after"] == "cursor"
421+
422+
def test_handles_empty_params(self):
423+
params = {"limit": 10, "order": "desc"}
424+
result = _prepare_request_params(params)
425+
assert result == {"limit": 10, "order": "desc"}

0 commit comments

Comments
 (0)