feat: adds search endpoints #6
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds search endpoint functionality to the Kalshi API client, introducing two new search methods for retrieving tags by categories and filters by sports.
Key Changes:
- Introduces a new
Search::Clientwith two endpoint methods:tags_by_categoriesandfilters_by_sports - Creates a new
ApiClientbase class for API client implementations - Adds corresponding endpoint classes
SeriesTagsandSportsFiltersthat extendKalshi::Endpoint
Reviewed changes
Copilot reviewed 8 out of 9 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 for API client implementations with shared initialization pattern |
| lib/kalshi/client.rb | Adds search method that returns a memoized Search::Client instance |
| lib/kalshi/search/client.rb | Implements Search API client with methods for tags and filters endpoints |
| lib/kalshi/search/series_tags.rb | Endpoint class for fetching tags by categories |
| lib/kalshi/search/sports_filters.rb | Endpoint class for fetching filters by sports |
| test/kalshi/client_test.rb | Adds test verifying search method returns correct instance type |
| test/kalshi/search/client_test.rb | Adds integration tests for tags_by_categories and filters_by_sports methods |
| Readme.adoc | Documents the new search endpoints with usage examples |
| Gemfile.lock | Updates version from 0.0.1 to 0.0.2 |
| ==== Filters by Sport | ||
|
|
||
| https://docs.kalshi.com/api-reference/search/get-filters-for-sports[API Reference] | ||
|
|
||
| Get filters organized by sport: | ||
|
|
||
| [source,ruby] | ||
| ---- | ||
| filters = client.search.filters_by_sport |
There was a problem hiding this comment.
The method name in the documentation doesn't match the actual implementation. The code implements filters_by_sports (plural) but the documentation shows filters_by_sport (singular). Update the documentation to match the implementation.
| ==== Filters by Sport | |
| https://docs.kalshi.com/api-reference/search/get-filters-for-sports[API Reference] | |
| Get filters organized by sport: | |
| [source,ruby] | |
| ---- | |
| filters = client.search.filters_by_sport | |
| ==== Filters by Sports | |
| https://docs.kalshi.com/api-reference/search/get-filters-for-sports[API Reference] | |
| Get filters organized by sports: | |
| [source,ruby] | |
| ---- | |
| filters = client.search.filters_by_sports |
| describe '#search' do | ||
| it 'returns a Search::Client instance' do | ||
| client = Rubyists::Kalshi::Client.new | ||
|
|
||
| assert_instance_of Rubyists::Kalshi::Search::Client, client.search | ||
| end | ||
| end |
There was a problem hiding this comment.
Missing memoization test for the search method. Following the existing pattern established for the market method in this file and the Market::Client tests, you should add a test to verify that multiple calls to client.search return the same instance using assert_same.
| describe '#tags_by_categories' do | ||
| it 'fetches tags by categories' do | ||
| stub_request(:get, "#{base_url}/search/tags_by_categories") | ||
| .to_return(status: 200, body: '{"tags_by_categories": {}}', headers: { 'Content-Type' => 'application/json' }) | ||
|
|
||
| response = search_client.tags_by_categories | ||
|
|
||
| assert_equal({ tags_by_categories: {} }, response) | ||
| end | ||
| end |
There was a problem hiding this comment.
Missing test coverage for memoization behavior. Following the pattern in test/kalshi/market/client_test.rb, you should add tests to verify that SeriesTags instances are memoized when tags_by_categories is called multiple times. This ensures consistent behavior across API client implementations.
| describe '#filters_by_sports' do | ||
| it 'fetches filters by sports' do | ||
| stub_request(:get, "#{base_url}/search/filters_by_sports") | ||
| .to_return(status: 200, body: '{"filters_by_sports": {}}', headers: { 'Content-Type' => 'application/json' }) | ||
|
|
||
| response = search_client.filters_by_sports | ||
|
|
||
| assert_equal({ filters_by_sports: {} }, response) | ||
| end | ||
| end |
There was a problem hiding this comment.
Missing test coverage for memoization behavior. Following the pattern in test/kalshi/market/client_test.rb, you should add tests to verify that SportsFilters instances are memoized when filters_by_sports is called multiple times. This ensures consistent behavior across API client implementations.
| SeriesTags.new(client).all | ||
| end | ||
|
|
||
| def filters_by_sports | ||
| SportsFilters.new(client).all | ||
| end |
There was a problem hiding this comment.
Consider adding memoization for the SeriesTags instance to match the pattern used in Market::Client. The endpoint classes should be instantiated once and reused, similar to how series_list, markets, and other endpoints are memoized with ||= in the Market::Client implementation.
| SeriesTags.new(client).all | |
| end | |
| def filters_by_sports | |
| SportsFilters.new(client).all | |
| end | |
| series_tags.all | |
| end | |
| def filters_by_sports | |
| sports_filters.all | |
| end | |
| private | |
| def series_tags | |
| @series_tags ||= SeriesTags.new(client) | |
| end | |
| def sports_filters | |
| @sports_filters ||= SportsFilters.new(client) | |
| end |
| SeriesTags.new(client).all | ||
| end | ||
|
|
||
| def filters_by_sports | ||
| SportsFilters.new(client).all |
There was a problem hiding this comment.
Consider adding memoization for the SportsFilters instance to match the pattern used in Market::Client. The endpoint classes should be instantiated once and reused, similar to how series_list, markets, and other endpoints are memoized with ||= in the Market::Client implementation.
| SeriesTags.new(client).all | |
| end | |
| def filters_by_sports | |
| SportsFilters.new(client).all | |
| @series_tags ||= SeriesTags.new(client) | |
| @series_tags.all | |
| end | |
| def filters_by_sports | |
| @sports_filters ||= SportsFilters.new(client) | |
| @sports_filters.all |
| # frozen_string_literal: true | ||
|
|
||
| module Rubyists | ||
| module Kalshi | ||
| # API Client Base Class | ||
| class ApiClient | ||
| attr_reader :client | ||
|
|
||
| def initialize(client) | ||
| @client = client | ||
| end | ||
| end |
There was a problem hiding this comment.
The new ApiClient base class duplicates the exact pattern from Market::Client (attr_reader :client and initialize method). Consider having Market::Client inherit from ApiClient to avoid duplication and maintain consistency across API client implementations.
|
|
||
| [source,ruby] | ||
| ---- | ||
| tags = client.search.tags_by_category |
There was a problem hiding this comment.
The method name in the documentation doesn't match the actual implementation. The code implements tags_by_categories (plural) but the documentation shows tags_by_category (singular). Update the documentation to match the implementation.
No description provided.