Skip to content

Conversation

@albertsola
Copy link
Contributor

  • Disabled camel_killer_box
  • Renamed Model._resource_data to Model._box for better naming clarity
  • Added models representation

@github-actions
Copy link

github-actions bot commented Nov 6, 2025

Warnings
⚠️

This PR contains 2 commits.

Please squash them into a single commit to keep the git history clean and easy to follow.

Multiple commits are acceptable only in the following cases:

  1. One commit is a technical refactoring, and another introduces business logic changes.
  2. You are doing a complex multi-step refactoring (although in this case we still recommend splitting it into separate PRs).

✅ Found Jira issue key in the title: MPT-15233

Generated by 🚫 dangerJS against c0f6b5f

- Disabled camel_killer_box
- Renamed Model._resource_data to Model._box for better naming clarity
- Added models representation
@albertsola albertsola force-pushed the MPT-15233/API-client-payload-casing-must-not-be-converted-to-snake-case-when-calling-to_dict branch from 0c9bb4d to d13d35c Compare November 6, 2025 14:57
Comment on lines 75 to 105
def test_case_conversion():
resource_data = {"id": "abc-123", "FullName": "Alice Smith"}

resource = Model(resource_data)

assert resource.FullName == "Alice Smith"
assert resource.to_dict() == resource_data
with pytest.raises(AttributeError):
resource.full_name # noqa: B018

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@d3rky @ruben-sebrango @svazquezco @robcsegal

can we review this is the expected behaviour?

assert resource.FullName == "Alice Smith"
assert resource.to_dict() == resource_data
with pytest.raises(AttributeError):
resource.full_name # noqa: B018
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not follow the ruff recommendations instead of adding the noqa? ->

Image

@albertsola albertsola marked this pull request as draft November 7, 2025 09:16

class Model:
class MptBox(Box):
"""python-box that preserves camelCase keys when converted to json."""
Copy link
Contributor

Choose a reason for hiding this comment

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

I would also describe here that the key_mapping keyword is reserved for now


assert resource.full_name == "Alice Smith"
assert resource.to_dict() == resource_data
with pytest.raises(AttributeError):
Copy link
Contributor

Choose a reason for hiding this comment

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

Honestly speaking I would split it to two functions, just because you are checking here different things --> key-access and to_dict method

assert resource.to_dict() == expected_resource_data

with pytest.raises(AttributeError):
_ = resource.contact.FullName # noqa: WPS122
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't need this ignore now :-)

- Disabled camel_killer_box
- Renamed Model._resource_data to Model._box for better naming clarity
- Added models representation
@d3rky d3rky force-pushed the MPT-15233/API-client-payload-casing-must-not-be-converted-to-snake-case-when-calling-to_dict branch from b1cb96d to c0f6b5f Compare November 10, 2025 11:01
@sonarqubecloud
Copy link

…ing-must-not-be-converted-to-snake-case-when-calling-to_dict' into MPT-15233/API-client-payload-casing-must-not-be-converted-to-snake-case-when-calling-to_dict
@albertsola
Copy link
Contributor Author

This is a draft PR, I have substantial WIP changes to push.

@albertsola albertsola closed this Nov 10, 2025
@d3rky d3rky deleted the MPT-15233/API-client-payload-casing-must-not-be-converted-to-snake-case-when-calling-to_dict branch November 19, 2025 09:20
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.

4 participants