Skip to content

fix(scim): keep lists of plain values flat in snake_cased_body #1973

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kwy404 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix(scim): keep lists of plain values flat in snake_cased_body #1973

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c372eaa
fix(scim): keep lists of plain values flat in snake_cased_body
kwy404 Sep 26, 2026
9f14b6c
Merge branch 'main' into fix-scim-snake-case-list-values
WilliamBergamin Oct 21, 2026
7f59173
test(scim): assert the response body keeps schemas flat
kwy404 Oct 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion slack_sdk/scim/v1/internal_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ def _convert_dict_keys(
)
result_dict[new_key].append(new_element)
else:
result_dict[new_key].append(_create_copy(original_value))
result_dict[new_key].append(_create_copy(element))

@WilliamBergamin WilliamBergamin Oct 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

praise: Nice catch, this is the right fix. Copying element instead of original_value keeps the list flat and leaves the list-of-dicts branch untouched.

else:
result_dict[new_key] = _create_copy(original_value)
return result_dict
Expand Down
4 changes: 4 additions & 0 deletions tests/slack_sdk/scim/test_internals.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,7 @@ def test_snake_cased(self):
result = _to_snake_cased(json.loads(response_body))
self.assertEqual(result["start_index"], 1)
self.assertIsNotNone(result["resources"][0]["id"])

def test_snake_cased_list_of_plain_values(self):
result = _to_snake_cased({"schemas": ["urn:scim:schemas:core:1.0", "urn:scim:schemas:extension:enterprise:1.0"]})
self.assertEqual(result["schemas"], ["urn:scim:schemas:core:1.0", "urn:scim:schemas:extension:enterprise:1.0"])

@WilliamBergamin WilliamBergamin Oct 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: This covers the helper well. Could we also pin the user-facing symptom? tests/slack_sdk/scim/test_response.py already builds a SearchUsersResponse from a body with "schemas": ["urn:scim:schemas:core:1.0"], so one assertion there would do it:

user = response.users[0]
self.assertEqual(user.schemas, ["urn:scim:schemas:core:1.0"])

That way a future change to snake_cased_body or the models can't bring the nesting back unnoticed.

@kwy404 kwy404 Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in 7f59173. One note: user.schemas passes even without the fix, because lists nested inside Resources items are copied as is and never reach that branch. The top level schemas of the same response does, so I asserted on response.snake_cased_body["schemas"] instead. It fails on main and passes with the fix.

1 change: 1 addition & 0 deletions tests/slack_sdk/scim/test_response.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ def test_snake_cased(self):
)
)
user = response.users[0]
self.assertEqual(response.snake_cased_body["schemas"], ["urn:scim:schemas:core:1.0"])
self.assertIsNotNone(user.unknown_fields.get("new_attribute"))
# the unknown fields need to be also camel-cased
self.assertIsNotNone(user.to_dict().get("newAttribute"))