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 remaining stations endpoints #20

Merged
merged 10 commits into from
Jun 3, 2020
Merged
Show file tree
Hide file tree
Changes from 6 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.
* [#20](https://github.com/dblock/open-weather-ruby-client/pull/20): Add remaining stations endpoints - [@wasabigeek](https://github.com/wasabigeek).
Copy link
Owner

Choose a reason for hiding this comment

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

Combine these into [#20](), [#19](), ...: Add support for weather stations - ...

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

Expand Down
30 changes: 30 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ Unlike other clients, including [open-weather](https://github.com/coderhs/ruby_o
- [Stations](#stations)
- [Register a Station](#register-a-station)
- [List Stations](#list-stations)
- [Get Station](#get-station)
- [Update Station](#update-station)
- [Delete Station](#delete-station)
- [Configuration](#configuration)
- [Units](#units)
- [Converting Temperature](#converting-temperature)
Expand Down Expand Up @@ -222,6 +225,33 @@ To list all stations, you can call the client method:
data = client.list_stations # => Array[OpenWeather::Models::Station]
```

#### Get Station

To get a stations, you can call the client method:
Copy link
Owner

Choose a reason for hiding this comment

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

nit: remove "you can", it goes without saying :) same below

```ruby
data = client.get_station('5ed2118acca8ce0001f1aeg1') # => OpenWeather::Models::Station
Copy link
Owner

Choose a reason for hiding this comment

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

nit: because you're not using data, just remove it, so client.get ..., more below like this

```

#### Update Station

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

Choose a reason for hiding this comment

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

Put model.register! on its own line.

model.update!(external_id: 'SF_TEST002')
model.external_id # => 'SF_TEST002'
```

#### Delete Station

To delete a station, you can call the client method:
```ruby
data = client.delete_station('5ed2118acca8ce0001f1aeg1') # => nil
```

## Configuration

You can configure client options, globally.
Expand Down
13 changes: 13 additions & 0 deletions lib/open_weather/endpoints/stations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,19 @@ def register_station(options = {})
def list_stations
get('stations').map { |data| OpenWeather::Models::Station.new(data) }
end

def get_station(id)
OpenWeather::Models::Station.new(get("stations/#{id}"))
end

def update_station(id, options = {})
OpenWeather::Models::Station.new(put("stations/#{id}", options))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure why, but it seems like the API requires external_id to be specified 🤷

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also, I could potentially raise an error if id was not defined, do you have a suggestion on how to define / where to keep errors?

Copy link
Owner

Choose a reason for hiding this comment

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

If a parameter is missing, raise ArgumentError, generally I don't specialize errors unless they can happen on all APIs (e.g. incorrect HTTP proxy)

end

def delete_station(id)
delete("stations/#{id}")
nil
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@dblock what would you suggest to return here? Currently it the delete method returns as empty string (response.body).

Copy link
Owner

Choose a reason for hiding this comment

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

I think nil is fine.

end
end
end
end
7 changes: 7 additions & 0 deletions lib/open_weather/models/station.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@ def register!

self
end

def update!(attributes)
data = OpenWeather::Client.new.update_station(id, attributes)
update_attributes!(data)

self
end
end
end
end
40 changes: 40 additions & 0 deletions spec/fixtures/open_weather/stations/delete_station_success.yml

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

44 changes: 44 additions & 0 deletions spec/fixtures/open_weather/stations/get_station_success.yml

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

44 changes: 44 additions & 0 deletions spec/fixtures/open_weather/stations/update_station_success.yml

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

20 changes: 20 additions & 0 deletions spec/lib/open_weather/models/station_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,24 @@
expect(result).to have_attributes(create_attributes.merge(id: '5ed21a12cca8ce0001f1aef1'))
end
end

describe '.update!' do
let(:update_attributes) do
{
external_id: 'SF_TEST002',
name: 'San Francisco Test Station 2'
}
end

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

it 'registers a station via the Client', vcr: { cassette_name: 'stations/update_station_success' } do
model = OpenWeather::Models::Station.new(id: '5ed21311cca8ce0001f1aef0')
result = model.update!(update_attributes)
expect(result.object_id).to eq(model.object_id)
expect(result).to have_attributes(update_attributes)
end
end
end
36 changes: 35 additions & 1 deletion spec/open_weather/endpoints/stations_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
end

describe '#list_stations' do
it 'registers a station', vcr: { cassette_name: 'stations/list_stations_success' } do
it 'retrieves all stations', vcr: { cassette_name: 'stations/list_stations_success' } do
data = client.list_stations
expect(data).to be_a(Array)
expect(data.size).to eq(2)
Expand All @@ -42,4 +42,38 @@
)
end
end

describe '#get_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.

should this be context?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh I’ve been following this convention http://www.betterspecs.org/#describe (describe for the method under test, context for a scenario)

Copy link
Owner

Choose a reason for hiding this comment

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

I've been schooled ;)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

They’re aliases so all good either way!

it 'retrieves a station', vcr: { cassette_name: 'stations/get_station_success' } do
data = client.get_station('5ed21311cca8ce0001f1aef0')
expect(data).to be_a(OpenWeather::Models::Station)
expect(data).to have_attributes(
id: '5ed21311cca8ce0001f1aef0',
external_id: 'SF_TEST001',
name: 'San Francisco Test Station',
latitude: 37.76,
longitude: -122.43,
altitude: 150
)
end
end

describe '#update_station' do
it 'updates a station', vcr: { cassette_name: 'stations/update_station_success' } do
update_attributes = {
external_id: 'SF_TEST002',
name: 'San Francisco Test Station 2'
}
data = client.update_station('5ed21311cca8ce0001f1aef0', update_attributes)
expect(data).to be_a(OpenWeather::Models::Station)
expect(data).to have_attributes(update_attributes)
end
end

describe '#delete_station' do
it 'deletes a station', vcr: { cassette_name: 'stations/delete_station_success' } do
data = client.delete_station('5ed21311cca8ce0001f1aef0')
expect(data).to be_nil
end
end
end