Skip to content

fix: renamed #get_without_prefix to #get_url for clarity #11

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.

Sign up for GitHub

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

Merged
bougyman merged 1 commit into from
Jan 5, 2026
Merged

fix: renamed #get_without_prefix to #get_url for clarity #11

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Gemfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
PATH
remote: .
specs:
kalshi (0.0.3)
kalshi (0.0.4)
dry-cli
dry-configurable
dry-container
Expand Down Expand Up @@ -230,7 +230,7 @@ CHECKSUMS
httpx (1.7.0) sha256=e219689555951f9c13c40862da120cdd2535e1454189bed589f04d7059557154
io-console (0.8.2) sha256=d6e3ae7a7cc7574f4b8893b4fca2162e57a825b223a177b7afa236c5ef9814cc
json (2.18.0) sha256=b10506aee4183f5cf49e0efc48073d7b75843ce3782c68dbeb763351c08fd505
kalshi (0.0.3)
kalshi (0.0.4)
language_server-protocol (3.17.0.5) sha256=fd1e39a51a28bf3eec959379985a72e296e9f9acfce46f6a79d31ca8760803cc
lint_roller (1.1.0) sha256=2c0c845b632a7d172cb849cc90c1bce937a28c5c8ccccb50dfd46a485003cc87
logger (1.7.0) sha256=196edec7cc44b66cfb40f9755ce11b392f21f7967696af15d274dde7edff0203
Expand Down
27 changes: 24 additions & 3 deletions lib/kalshi/client.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

require 'httpx'
require 'json'
require 'uri'

module Rubyists
module Kalshi
Expand All @@ -19,13 +20,33 @@ def initialize(base_url: Kalshi.config.base_url)
.with(origin: base_url)
end

# Get response from a path, adding the base_url,
# and a prefix, if set on the client.
#
# see #full_path for details
#
# @param path [String] The URL path
#
# @return [Hash] The parsed JSON response
def get(path, params: {})
get_without_prefix(full_url(path), params:)
get_url(full_url(path), params:)
end

def get_without_prefix(path, params: {})
response = @http.get(path, params:)
# Get response from a URL
# Must pass a full URL, including scheme (http/https), host, etc.
#
# @param path [String] The full URL path
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parameter is named 'path' but the method now expects a full URL. The parameter name should be 'url' to match the method name and its documentation.

Suggested change
# @param path [String] The full URL path
# @param url [String] The full URL

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation says '@param path [String] The full URL path' but the parameter represents a complete URL, not a path. It should read '@param url [String] The full URL' to accurately describe what's expected.

Suggested change
# @param path [String] The full URL path
# @param url [String] The full URL

Copilot uses AI. Check for mistakes.
#
# @return [Hash] The parsed JSON response
def get_url(url, params: {})
uri = URI.parse(url)
raise ArgumentError, 'URL must be http or https' unless %w[http https].include?(uri.scheme)

response = @http.get(url, params:)
handle_response(response)
rescue ArgumentError => e
Comment on lines +43 to +47
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error raised is ArgumentError but caught and re-raised as Error. This creates inconsistency - URI.parse raises URI::InvalidURIError for invalid URIs, not ArgumentError. The custom ArgumentError on line 43 won't be caught by the rescue on line 47 if URI.parse fails first. Consider rescuing URI::InvalidURIError and StandardError to handle both validation scenarios properly.

Suggested change
raise ArgumentError, 'URL must be http or https' unless %w[http https].include?(uri.scheme)
response = @http.get(url, params:)
handle_response(response)
rescue ArgumentError => e
raise URI::InvalidURIError, 'URL must be http or https' unless %w[http https].include?(uri.scheme)
response = @http.get(url, params:)
handle_response(response)
rescue URI::InvalidURIError => e

Copilot uses AI. Check for mistakes.
logger.error('Invalid URL', url:, exception: e)
raise Error, "Invalid URL: #{e.message}"
end

def market
Expand Down
29 changes: 29 additions & 0 deletions test/kalshi/client_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,35 @@
end
end

describe '#get_url' do
it 'fetches from a full URL' do
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following BetterSpecs guidelines, test descriptions should focus on behavior rather than implementation. Consider renaming to 'returns data from a full URL' to describe what the method accomplishes rather than how it works.

Copilot generated this review using guidance from repository custom instructions.
url = 'https://example.com/api/resource'
stub_request(:get, url)
.to_return(status: 200, body: '{"data": "value"}', headers: { 'Content-Type' => 'application/json' })

client = Rubyists::Kalshi::Client.new
response = client.get_url(url)

assert_equal({ data: 'value' }, response)
end

it 'raises Error for invalid URL scheme' do
client = Rubyists::Kalshi::Client.new
error = assert_raises(Rubyists::Kalshi::Error) do
client.get_url('ftp://example.com')
end
assert_match(/Invalid URL/, error.message)
end

it 'raises Error for invalid URL format' do
client = Rubyists::Kalshi::Client.new
error = assert_raises(Rubyists::Kalshi::Error) do
client.get_url('not_a_url')
end
assert_match(/Invalid URL/, error.message)
end
Comment on lines +33 to +47
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following BetterSpecs guidelines, use context blocks to group related test cases by scenario. Consider wrapping these validation tests in a 'context "with invalid URLs"' block to make the test organization clearer.

Copilot generated this review using guidance from repository custom instructions.
end

describe '#market' do
it 'returns a Market::Client instance' do
client = Rubyists::Kalshi::Client.new
Expand Down