dev: Implemented workspace roles endpoint and pagination support#54
Conversation
📝 WalkthroughWalkthroughAdds a ChangesRoles API Resource
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plane/models/roles.py`:
- Around line 9-11: Update the docstring in roles.py for the role model (the
class that defines id/label/slug) so it says the extra API fields are accepted
and preserved for forward compatibility, not ignored. Keep the note that slug is
the stable identifier to use in code, and make the wording consistent with the
model’s behavior and any typed extras handling in the Role class.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d6f65329-685a-417d-a680-484cf6bc8d2c
📒 Files selected for processing (5)
README.mdplane/api/roles.pyplane/client/plane_client.pyplane/models/roles.pytests/unit/test_roles.py
| The API returns additional fields (``id``, ``permissions``, ``level``, | ||
| ``member_count``, ...); only the three below are modeled and the rest are | ||
| ignored. ``slug`` is the stable identifier to use in code, but it is **not** |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor
🧩 Analysis chain
🌐 Web query:
In Pydantic v2.4, when model_config uses ConfigDict(extra="allow"), are unknown fields preserved and included in model_dump by default?
💡 Result:
Yes, in Pydantic v2.4 (and throughout the v2 series), when a model is configured with ConfigDict(extra="allow"), unknown fields are preserved and included in the output of model_dump by default [1][2][3]. When extra="allow" is set, any data provided to the model that does not correspond to a defined field is stored in a special dictionary attribute called pydantic_extra [1][2]. During serialization, Pydantic's model_dump method automatically includes the contents of pydantic_extra alongside the standard fields [1][2][3]. Example behavior: from pydantic import BaseModel, ConfigDict class Model(BaseModel): x: int model_config = ConfigDict(extra='allow') m = Model(x=1, y='a') assert m.model_dump == {'x': 1, 'y': 'a'} # Unknown field 'y' is included [1][3] assert m.pydantic_extra == {'y': 'a'} # Extra data is stored here [1][2] Note that while this is the standard behavior for model_dump, recent discussions in the Pydantic ecosystem have highlighted that this integration can sometimes be complex when using model_construct or specific serialization overrides [4][5]. However, under normal instantiation and serialization, these extra fields are included [1][2][3].
Citations:
- 1: https://pydantic.dev/docs/validation/latest/concepts/models/
- 2: https://pydantic.com.cn/en/concepts/models/
- 3: https://pydantic.dev/docs/validation/2.8/concepts/models/
- 4: Unexpected inconsistent behaviour when using 'model_construct' pydantic/pydantic#8266
- 5: Extra fields not dumped by
model_dumppydantic/pydantic#12937
Docstring should say extras are accepted, not ignored
plane/models/roles.py:9-11 — The API returns additional fields (id, permissions, level, member_count, ...); only the three below are explicitly typed, and the rest are accepted and preserved for forward compatibility. slug is the stable identifier to use in code, but it is not ...
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plane/models/roles.py` around lines 9 - 11, Update the docstring in roles.py
for the role model (the class that defines id/label/slug) so it says the extra
API fields are accepted and preserved for forward compatibility, not ignored.
Keep the note that slug is the stable identifier to use in code, and make the
wording consistent with the model’s behavior and any typed extras handling in
the Role class.
3 participants
Description
Implemented support for workspace roles APIs and added pagination handling in the Plane SDK.
Changes
Type of Change
Test Scenarios
Summary by CodeRabbit
New Features
Tests