Skip to content
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

Add register stations #18

Merged
merged 22 commits into from
Jun 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
### 0.2.1 (Next)

* Your contribution here.
* [#18](https://github.com/dblock/open-weather-ruby-client/pull/18): Add register_station endpoint - [@wasabigeek](https://github.com/wasabigeek).

### 0.2.0 (2020/05/17)

Expand Down
21 changes: 21 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ Unlike other clients, including [open-weather](https://github.com/coderhs/ruby_o
- [One Call](#one-call)
- [Current and Forecast Weather](#current-and-forecast-weather)
- [Historical Weather](#historical-weather)
- [Stations](#stations)
- [Register a Station](#register-a-station)
- [Configuration](#configuration)
- [Units](#units)
- [Converting Temperature](#converting-temperature)
Expand Down Expand Up @@ -193,6 +195,25 @@ data.current # => OpenWeather::Models::OneCall::CurrentWeather
data.hourly # => Array[OpenWeather::Models::OneCall::HourlyWeather]
```

### Stations

The [Stations API](https://openweathermap.org/stations) lets your manage personal weather stations and measurements.

#### Register a Station

To register a station, you can call the client method:
```ruby
data = client.register_station(external_id: 'SF_TEST001', ...) # => OpenWeather::Models::Station
data.id # => '5ed2118acca8ce0001f1aeg1'
data.external_id # => 'SF_TEST001'
```
Alternatively, call `register!` on an instance of `Station`:
```ruby
model = OpenWeather::Models::Station.new(external_id: 'SF_TEST001', ...)
model.register!
model.id # => '5ed2118acca8ce0001f1aeg1'
```

## Configuration

You can configure client options, globally.
Expand Down
1 change: 1 addition & 0 deletions lib/open_weather/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ class Client
include Request
include Endpoints::Current
include Endpoints::OneCall
include Endpoints::Stations

attr_accessor(*Config::ATTRIBUTES)

Expand Down
5 changes: 4 additions & 1 deletion lib/open_weather/connection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ def headers
def connection
@connection ||= begin
options = {
headers: headers.merge('Accept' => 'application/json; charset=utf-8')
headers: headers.merge(
'Accept' => 'application/json; charset=utf-8',
'Content-Type' => 'application/json'
)
}

options[:headers]['User-Agent'] = user_agent if user_agent
Expand Down
1 change: 1 addition & 0 deletions lib/open_weather/endpoints.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@

require_relative 'endpoints/current'
require_relative 'endpoints/one_call'
require_relative 'endpoints/stations'
11 changes: 11 additions & 0 deletions lib/open_weather/endpoints/stations.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# frozen_string_literal: true

module OpenWeather
module Endpoints
module Stations
def register_station(options = {})
OpenWeather::Models::Station.new(post('stations', options))
end
end
end
end
1 change: 1 addition & 0 deletions lib/open_weather/models.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,4 @@
require_relative 'models/list'
require_relative 'models/city'
require_relative 'models/one_call'
require_relative 'models/station'
3 changes: 2 additions & 1 deletion lib/open_weather/models/model.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ class Model < Hashie::Trash
attr_reader :options

def initialize(args = nil, options = {})
super args
transformed_args = args.respond_to?(:transform_keys) ? args.transform_keys(&:to_s) : args
Copy link
Owner

Choose a reason for hiding this comment

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

Care to explain?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

A symbol-keyed hash was “ignored” by the model, I think because a Hashie::Dash property will account for whether it’s defined as a string or symbol (though there didn’t seem to be an issue retrieving the property via a method)

Copy link
Owner

Choose a reason for hiding this comment

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

it's possible that the model class needs a merge initializer instead. Either way make sure to have a spec for this and we can fix it later.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Finding this hard to test on Model itself, would it be sufficient to test Station for now?

Copy link
Owner

Choose a reason for hiding this comment

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

Yes.

super transformed_args
@options = { units: OpenWeather.config.units }.merge(options || {})
end

Expand Down
24 changes: 24 additions & 0 deletions lib/open_weather/models/station.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# frozen_string_literal: true

module OpenWeather
module Models
class Station < Model
property 'id', from: 'ID' # internal identifier for the station
property 'external_id' # external identifier for the station
property 'name' # name of the station
property 'latitude' # geographical coordinates of the location (latitude)
property 'longitude' # geographical coordinates of the location (longitude)
property 'altitude' # height of station above sea level
property 'created_at' # timestamp when station was created
property 'updated_at' # timestamp when station was updated
property 'rank' # rank of station

def register!
data = OpenWeather::Client.new.register_station(to_h)
Copy link
Owner

Choose a reason for hiding this comment

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

For future update, we can save the extra instance construction with OpenWeather::Client.new.post('stations', options). I think that's OK to duplicate.

update_attributes!(data)

self
end
end
end
end
3 changes: 2 additions & 1 deletion lib/open_weather/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ def request(method, path, options)
request.url(path, options)
when :post, :put
request.path = path
request.body = options unless options.empty?
request.params = { appid: options.delete(:appid) }
request.body = options.to_json unless options.empty?
end
request.options.merge!(options.delete(:request)) if options.key?(:request)
end
Expand Down
44 changes: 44 additions & 0 deletions spec/fixtures/open_weather/stations/register_success.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 30 additions & 0 deletions spec/lib/open_weather/models/station_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# frozen_string_literal: true

require 'spec_helper'

RSpec.describe OpenWeather::Models::Station do
Copy link
Owner

Choose a reason for hiding this comment

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

Probably shorter to use the same VCR here. The problem with the double is that it will continue working even if we break the API call.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Curious, since that client method is unit-tested already, wouldn't it be sufficient to unit-test this as well (check that we call client with the right params)?

Copy link
Owner

Choose a reason for hiding this comment

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

It's sufficient, but implies behavior and IMHO hides bugs. I've debugged my fair share of double's to avoid them, more often than to use them. That said no strong opinions.

include_context 'API client', endpoint: 'https://api.openweathermap.org/data/3.0'

describe '.register!' do
let(:create_attributes) do
{
external_id: 'SF_TEST001',
name: 'San Francisco Test Station',
latitude: 37.76,
longitude: -122.43,
altitude: 150
}
end

before do
allow(OpenWeather::Client).to receive(:new).and_return(client)
end

it 'registers a station via the Client', vcr: { cassette_name: 'stations/register_success' } do
model = described_class.new(create_attributes)
result = model.register!
expect(result.object_id).to eq(model.object_id)
expect(result).to have_attributes(create_attributes.merge(id: '5ed21a12cca8ce0001f1aef1'))
end
end
end
26 changes: 26 additions & 0 deletions spec/open_weather/endpoints/stations_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# frozen_string_literal: true

require 'spec_helper'

RSpec.describe 'stations' do
include_context 'API client', endpoint: 'https://api.openweathermap.org/data/3.0'

it 'registers a station', vcr: { cassette_name: 'stations/register_success' } do
data = client.register_station(
external_id: 'SF_TEST001',
name: 'San Francisco Test Station',
latitude: 37.76,
longitude: -122.43,
altitude: 150
)
expect(data).to be_a(OpenWeather::Models::Station)
expect(data).to have_attributes(
Copy link
Owner

Choose a reason for hiding this comment

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

As long as have_attributes starts failing if I add an extra attribute, this works, but it is inconsistent with other tests. Not sure if I care about the latter much, just thinking out loud.

Copy link
Collaborator Author

@wasabigeek wasabigeek Jun 1, 2020

Choose a reason for hiding this comment

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

It does fail, but perhaps with a more cryptic error:

  1) stations registers a station
     Failure/Error:
       expect(data).to have_attributes(
         ...
         wazzoo: 'test'
       )

       expected {...} to respond to :wazzoo with 0 arguments

Copy link
Owner

Choose a reason for hiding this comment

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

So that's better, it lets us check all attributes. I wouldn't mind if all the other tests changed ;)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So that's better, it lets us check all attributes. I wouldn't mind if all the other tests changed ;)

Can I open a separate PR for that 🤣

id: '5ed21a12cca8ce0001f1aef1',
external_id: 'SF_TEST001',
name: 'San Francisco Test Station',
latitude: 37.76,
longitude: -122.43,
altitude: 150
)
end
end
3 changes: 2 additions & 1 deletion spec/support/shared/api_client.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
# frozen_string_literal: true

RSpec.shared_context 'API client', shared_context: :metadata do
RSpec.shared_context 'API client', shared_context: :metadata do |params|
before do
OpenWeather::Config.reset
OpenWeather::Config.endpoint = params[:endpoint] if params&.key?(:endpoint)
Copy link
Owner

Choose a reason for hiding this comment

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

For a future PR, I'd like to make it future proof able to write OpenWeather::Client.configure(params) here.

end
let(:client) { OpenWeather::Client.new(api_key: ENV['OPEN_WEATHER_API_KEY'] || 'api-key') }
end