fix: correct path to filter_by_sport, adds ApiClient helper #8
Conversation
There was a problem hiding this comment.
Pull request overview
This PR corrects API endpoint paths and introduces an ApiClient base class to automatically manage URL prefixes for different API namespaces. The changes fix inconsistencies where endpoint paths incorrectly included the namespace prefix.
- Corrected endpoint paths by removing duplicate namespace prefixes (
search/filters_by_sport→filters_by_sport) - Introduced
ApiClientbase class with automatic prefix extraction from module hierarchy - Refactored
Client.getto use the new prefix mechanism viafull_url
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
lib/kalshi/api_client.rb |
Adds new base class with automatic prefix detection from class namespace |
lib/kalshi/client.rb |
Refactors get method to use full_url with prefix; adds get_without_prefix helper |
lib/kalshi/search/client.rb |
Renames method from filters_by_sports to filters_by_sport |
lib/kalshi/search/sports_filters.rb |
Corrects path from search/filters_by_sports to filters_by_sport |
lib/kalshi/search/series_tags.rb |
Corrects path from search/tags_by_categories to tags_by_categories |
test/kalshi/api_client_test.rb |
Adds basic test for prefix setter functionality |
test/kalshi/search/client_test.rb |
Updates test to match renamed method filters_by_sport |
Gemfile.lock |
Version bump from 0.0.2 to 0.0.3 |
| def get(path, params: {}) | ||
| response = @http.get(full_url(path), params:) | ||
| get_without_prefix(full_url(path), params:) | ||
| end | ||
|
|
||
| def get_without_prefix(path, params: {}) | ||
| response = @http.get(path, params:) | ||
| handle_response(response) | ||
| end |
There was a problem hiding this comment.
The new get_without_prefix method and the refactored get method lack test coverage. Consider adding tests that verify the prefix is correctly applied when calling get, and that get_without_prefix bypasses the prefix logic.
| @client = client | ||
| client.prefix = self.class.prefix |
There was a problem hiding this comment.
The initialize method modifies the shared client instance by setting its prefix. This creates a side effect where if the same client instance is reused across multiple ApiClient subclasses, the prefix will be overwritten. Consider if this is the intended behavior or if each ApiClient should use its own client instance.
| @client = client | |
| client.prefix = self.class.prefix | |
| @client = client.dup | |
| @client.prefix = self.class.prefix |
| def initialize(client) | ||
| @client = client | ||
| client.prefix = self.class.prefix | ||
| end |
There was a problem hiding this comment.
The initialize method in ApiClient lacks test coverage. Consider adding a test that verifies the client is properly assigned and that the prefix is set on the client instance when an ApiClient subclass is instantiated.
No description provided.