fix: correct users get/update/delete to /api/v1/user?id=...
#110
Conversation
The Management API defines GET/PATCH/DELETE on `/api/v1/user` with `id`
as a query parameter (not `/api/v1/users/{user_id}`). This updates the
users endpoints to match the spec and prevents 404s.
WalkthroughUpdated ManagementClient endpoint definitions and method generation: users.get, users.update, and users.delete now use Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant ManagementClient
participant KindeAPI
Note over App,ManagementClient: Get user (updated)
App->>ManagementClient: get_user(id)
ManagementClient->>KindeAPI: GET /api/v1/user?id={id}
KindeAPI-->>ManagementClient: 200 JSON
ManagementClient-->>App: user object
rect rgba(200,230,255,0.25)
Note over App,ManagementClient: Update user (updated)
App->>ManagementClient: update_user(id, payload)
ManagementClient->>KindeAPI: PUT /api/v1/user?id={id} (application/json)
KindeAPI-->>ManagementClient: 200 JSON
ManagementClient-->>App: updated user
end
rect rgba(255,230,230,0.25)
Note over App,ManagementClient: Delete user (updated)
App->>ManagementClient: delete_user(id)
ManagementClient->>KindeAPI: DELETE /api/v1/user?id={id}
alt success
KindeAPI-->>ManagementClient: 204 No Content
ManagementClient-->>App: success
else error
KindeAPI-->>ManagementClient: 4xx/5xx
ManagementClient-->>App: error
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
✨ 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
|
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
kinde_sdk/management/management_client.py (1)
441-469: Always pass query params (incl. PATCH) and alias user_id→id; remove unused final_path.
- Ensures
idis encoded via the client serializer.- Keeps backward compatibility: positional first arg still works;
user_idkwarg is mapped toid.- Drops dead code (
final_path) that’s never used.Apply this diff:
@@ - # Handle query params or body data based on HTTP method - query_params = None - body = None - - if http_method in ('GET', 'DELETE'): - query_params = {k: v for k, v in kwargs.items() if v is not None} - else: - body = {k: v for k, v in kwargs.items() if v is not None} - - # FIXED: Use param_serialize to properly construct the full URL with host - # Handle query parameters by appending them to the path - final_path = formatted_path - if query_params and http_method in ('GET', 'DELETE'): - query_string = '&'.join([f"{k}={v}" for k, v in query_params.items() if v is not None]) - if query_string: - separator = '&' if '?' in final_path else '?' - final_path = f"{final_path}{separator}{query_string}" + # Handle query params or body data + query_params = None + body = None + + if http_method in ('GET', 'DELETE'): + query_params = {k: v for k, v in kwargs.items() if v is not None} + else: + body = {k: v for k, v in kwargs.items() if v is not None} + + # Users get/update/delete require an id in query string; accept id or user_id kwargs + # and also the first positional arg for back-compat. + if resource == 'users' and action in ('get', 'update', 'delete'): + positional_id = args[0] if args else None + canonical_id = kwargs.get('id', kwargs.get('user_id', positional_id)) + if canonical_id is not None: + query_params = (query_params or {}) + query_params['id'] = canonical_id + # Ensure id fields don't leak into body or duplicate in query + if body: + body.pop('id', None) + body.pop('user_id', None) + query_params.pop('user_id', None) @@ - method, url, header_params, body, post_params = self.api_client.param_serialize( + _serialized_method, url, header_params, body, post_params = self.api_client.param_serialize( method=http_method, resource_path=resource_path_for_serialize, # Use path without /api/v1 prefix - query_params=query_params if http_method in ('GET', 'DELETE') else None, + query_params=query_params or None, header_params={}, body=body if http_method not in ('GET', 'DELETE') else None )I can add focused unit tests for
get_user/update_user/delete_userto assert the final serialized URL carries?id=...and that kwargs don’t duplicate/bleed into the body. Want me to push those?
🧹 Nitpick comments (1)
kinde_sdk/management/management_client.py (1)
509-518: Docstring param name clarity.For users endpoints, consider documenting that
id(alias:user_id) can be passed positionally or via kwargs to avoid confusion after the switch to query param.
📜 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 (1)
kinde_sdk/management/management_client.py(1 hunks)
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…ect request serialization
- Add optional `query_keys` to API_ENDPOINTS signatures to declare which kwargs
must be lifted into the query string (e.g. `('PATCH', '/api/v1/user', ['id'])`).
- Update `_generate_methods` and `_create_api_method` to handle the 2- vs 3-tuple
endpoint forms and to route declared `query_keys` to `query_params`.
- Ensure `get_user` / `update_user` work with `/api/v1/user` by passing `id` as a
query parameter (no path templating required).
- Set default headers for write methods (`Content-Type` / `Accept` = application/json).
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
kinde_sdk/management/management_client.py (1)
436-457: Breaking change: positional args to get/update/delete user are now ignored. Map args → query_keys for back-compat and validate required keys.Previously,
get_user("abc")filled{user_id}. With/api/v1/user(no path placeholder), the current logic drops positionalargsand sends noid. Map positionalargsto declaredquery_keys(e.g.,['id']) and fail fast if required keys are missing. Also allow callers to pass_request_timeoutwithout leaking it into query/body.def api_method(*args, **kwargs) -> Dict[str, Any]: # Format path with any path parameters from args formatted_path = path if '{' in path and args: param_values = list(args) while '{' in formatted_path and param_values: start_idx = formatted_path.find('{') end_idx = formatted_path.find('}') if start_idx >= 0 and end_idx >= 0: formatted_path = formatted_path[:start_idx] + str(param_values.pop(0)) + formatted_path[end_idx + 1:] + # Allow method-level timeout without polluting query/body + _request_timeout = kwargs.pop('_request_timeout', None) + + # Back-compat: if no placeholders but we have declared query_keys, map positional args -> query keys + if '{' not in path and args and query_keys: + for i, key in enumerate(query_keys): + if i < len(args) and key not in kwargs: + kwargs[key] = args[i] + + # Validate required query keys when declared + if query_keys: + missing = [k for k in query_keys if kwargs.get(k) is None] + if missing: + raise ValueError(f"Missing required query parameter(s): {', '.join(missing)}") # Handle query/body split if http_method in ('GET', 'DELETE'): query_params = {k: v for k, v in kwargs.items() if v is not None} payload = None else: # Lift ONLY declared query_keys into the query string qset = set(query_keys or ()) query_params = {k: kwargs.pop(k) for k in list(kwargs) if k in qset and kwargs[k] is not None} # Remaining kwargs go to JSON body payload = {k: v for k, v in kwargs.items() if v is not None}And pass the timeout to the request (see comment below).
🧹 Nitpick comments (3)
kinde_sdk/management/management_client.py (3)
458-466: Remove dead manual query-string assembly; param_serialize already handles it.This block builds
final_pathbut it’s never used. Keeping it risks later double-encoding/duplication.- # Handle query parameters by appending them to the path - final_path = formatted_path - if query_params and http_method in ('GET', 'DELETE'): - query_string = '&'.join([f"{k}={v}" for k, v in query_params.items() if v is not None]) - if query_string: - separator = '&' if '?' in final_path else '?' - final_path = f"{final_path}{separator}{query_string}"
489-496: Honor caller-specified timeout.Wire the
_request_timeoutextracted in the method to avoid infinite hangs.- _request_timeout=None + _request_timeout=_request_timeout
522-522: Docstrings: prefer declared query_keys for parameter name (use id).Without placeholders, the docs currently say
user_id. Preferidwhenquery_keysexist to match the API.- param_name = path.split('{')[-1].split('}')[0] if '{' in path else f"{resource_singular}_id" + param_name = path.split('{')[-1].split('}')[0] if '{' in path else (query_keys[0] if query_keys else f"{resource_singular}_id")Apply the same change to the
updateanddeletebranches.Also applies to: 544-544, 556-556
📜 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 (1)
kinde_sdk/management/management_client.py(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
kinde_sdk/management/management_client.py (2)
kinde_sdk/management/api_client.py (1)
param_serialize(142-248)kinde_sdk/management/management_token_manager.py (1)
get_access_token(223-231)
🔇 Additional comments (2)
kinde_sdk/management/management_client.py (2)
396-402: 3-tuple endpoint support is clean and readable.Good extensibility; preserves 2-tuple handling and gracefully unpacks
query_keyswhen present.
471-477: Good use of param_serialize with host and query_params.The
/api/v1stripping is correct givenConfiguration(host=self.base_url)includes/api/v1.
|
Looks good, I am happy and accepting it |
2 participants
Explain your changes
This PR updates the
ManagementClientto align the usersget,update, anddeleteendpoints with the Management API spec:/api/v1/user?id={id}instead of/api/v1/users/{user_id}Fixes #89
Checklist
🛟 If you need help, consider asking for advice over in the Kinde community.