-
Notifications
You must be signed in to change notification settings - Fork 23
docs(management): update examples and README for corrected users endpoints #121
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.
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughDocumentation and example updates align ManagementClient user methods to use id instead of user_id and update create_user payload to use a nested profile object. Examples now call get_user(id=...). Management README references management_client.py and clarifies single-user endpoints use query parameter id. Changes
Sequence Diagram(s)Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
README_management_client.md (2)
89-93: Standardize method list parameter formattingFor consistency with other entries (e.g.,
get_role(role_id)), preferget_user(id),update_user(id, **data),delete_user(id)rather thanid=user_idin the methods list.-- `get_user(id=user_id)` - Get a specific user +- `get_user(id)` - Get a specific user -- `update_user(id=user_id, **data)` - Update a user +- `update_user(id, **data)` - Update a user -- `delete_user(id=user_id)` - Delete a user +- `delete_user(id)` - Delete a user
147-150: Create user payload update looks good; add a brief migration noteThe
profile.given_name/family_namestructure is clear. Consider a small “Migration” callout showing the old vs new payload to help users adjust quickly.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
README_management_client.md(3 hunks)examples/management_client_example.py(1 hunks)kinde_sdk/management/README.md(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-21T04:46:50.368Z
Learnt from: KomanRudden
PR: kinde-oss/kinde-python-sdk#101
File: kinde_sdk/auth/permissions.py:6-7
Timestamp: 2025-08-21T04:46:50.368Z
Learning: The kinde_sdk has both frontend and management API modules. The frontend permissions_api.py exists at kinde_sdk/frontend/api/permissions_api.py and contains PermissionsApi class. Using the management API instead of the frontend API for user permissions would be incorrect.
Applied to files:
kinde_sdk/management/README.md
🔇 Additional comments (4)
examples/management_client_example.py (1)
183-183: Switch to keyword id looks correctUsing
client.get_user(id=user_id)aligns with the updated single-user endpoints. LGTM.kinde_sdk/management/README.md (2)
10-10: Filename reference is accurate
management_client.pymatches the module path used elsewhere. LGTM.
145-145: Error handling example aligns with new signature
management.get_user(id=user_id)is consistent with the API note above. LGTM.README_management_client.md (1)
56-56: Quick Start uses the new id keyword correctly
get_user(id="user_id")reflects the updated API. LGTM.
| **Note on Users endpoints:** Single-user operations use a query parameter (`id`) with `/api/v1/user`, e.g.: | ||
| - `get_user(id)` | ||
| - `update_user(id, **data)` → `PATCH /api/v1/user?id=...` | ||
| - `delete_user(id)` → `DELETE /api/v1/user?id=...` | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix inconsistency with earlier path example for Users single-resource
This section states single-user ops use /api/v1/user?id=..., but the earlier “Dynamic Method Generation” example shows /users/{user_id}. This will confuse readers—please align the earlier snippet to the query-param style.
Apply this doc tweak to the earlier “API_ENDPOINTS” example to reflect single-user via query param:
- 'users': {
- 'list': ('GET', '/users'),
- 'get': ('GET', '/users/{user_id}'),
- # ...
- },
+ 'users': {
+ 'list': ('GET', '/users'),
+ # single-user operations use query param `id` with /user
+ 'get': ('GET', '/user'),
+ # ...
+ },And consider adding a short note near the snippet: “For users, supply id as a query parameter for GET/DELETE and in kwargs for PATCH/POST.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| **Note on Users endpoints:** Single-user operations use a query parameter (`id`) with `/api/v1/user`, e.g.: | |
| - `get_user(id)` | |
| - `update_user(id, **data)` → `PATCH /api/v1/user?id=...` | |
| - `delete_user(id)` → `DELETE /api/v1/user?id=...` | |
| 'users': { | |
| 'list': ('GET', '/users'), | |
| # single-user operations use query param `id` with /user | |
| 'get': ('GET', '/user'), | |
| # ... | |
| }, |
🤖 Prompt for AI Agents
In kinde_sdk/management/README.md around lines 67 to 71, the doc currently says
single-user operations use /api/v1/user?id=..., but an earlier "Dynamic Method
Generation" / API_ENDPOINTS example shows /users/{user_id}; update that earlier
snippet to use the query-parameter style (e.g., /api/v1/user with id passed as a
query param) so both sections are consistent, and add a short note next to the
adjusted snippet: "For users, supply id as a query parameter for GET/DELETE and
in kwargs for PATCH/POST."
Explain your changes
README_management_client.md:id=instead of positionaluser_idin examples.profileandidentitiesstructure.management_client_example.py:get_usernow useid=<user_id>.management/README.md:get_userusage to useid=user_id.Checklist
🛟 If you need help, consider asking for advice over in the Kinde community.