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

236 simple api rebased #290

Open
wants to merge 35 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
614ba53
Updated dev-envt config file
Dec 15, 2017
2274f4d
Create Adopted controller for API, and wrote initial auth/authorizati…
May 1, 2017
c007d1c
Add API route, and setting to allow for basic_auth
May 1, 2017
e1c2f24
Forgot to add the helper file
May 1, 2017
8b188b5
Edited authentication controller so that a session isn't created on t…
May 2, 2017
d16cbb3
Adopted Drains API controller test
May 2, 2017
6615a92
Trying to fix Travis CI errors
May 2, 2017
ac6c3f7
Added formatting for different content types, and edited fields to be…
May 2, 2017
b7d863a
(Finally realized I can run Rubocop on my local machine)
May 2, 2017
e2052d0
More controller tests
May 2, 2017
ffbe684
Add Faker gem in order to make sample users
Jun 20, 2017
cfee6da
Adding some user seeds
Jun 20, 2017
96aae96
Added a rake task for DEV so mock users can automatically adopt drain…
Jul 24, 2017
056ed5c
Passing rubocop
Jul 24, 2017
0f816bf
Changing !nil to nil... !nil evaluates to true
Jul 25, 2017
395ad25
Initial comments for cursor funcionality
Jul 26, 2017
227ee70
Adding Kaminari gem for pagination
Aug 14, 2017
9ce0b74
added pagination functionality to API
Aug 14, 2017
3ec42b8
Made is so that a CSV template is rendered with all results rather th…
Aug 14, 2017
b945bec
auto_adopt task comment for further clarification
Aug 14, 2017
0423a9c
Added a total_pages variable, changed default page to 1, and a adopte…
Dec 15, 2017
d822448
Updating versions
Dec 16, 2017
2c71f1b
Removed format options - JSON is the only format available now
Dec 16, 2017
32f2243
Updated pagination defaults
Dec 16, 2017
ca68a43
Uncommenting so I could add drains
Dec 16, 2017
c385883
Making this clearer, and printing out info
Dec 16, 2017
9bb9fdf
Response data tests for pagination counts and drain data
Dec 16, 2017
de24cf8
Adding seed data for API controller test that asserts page counts
Jan 9, 2018
89dcd50
Moving adopted_controller before_action filters for :index, to better…
Jan 9, 2018
eda7dcb
Using the Thing.adopted scope in place of a where clause
Jan 9, 2018
186b923
Removing API template for rendering CSV, since we are only allowing JSON
Jan 9, 2018
bb6afe4
Getting tests to pass
Jan 9, 2018
4c71b20
Changing require Rake to rake
Jan 9, 2018
5a92109
Trying to get the Github tests to pass - Unsure of the problem
Feb 5, 2018
d760550
Remove test file
Feb 5, 2018
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
2 changes: 2 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ gem 'local_time', '~> 2.0'
gem 'obscenity', '~> 1.0', '>= 1.0.2'
gem 'pg'
gem 'rails', '~> 4.2.10'
gem 'faker', '~> 1.7.0'
gem 'rails_admin', '~> 1.0'
gem 'validates_formatting_of', '~> 0.9.0'
gem 'kaminari', '~> 1.0'

gem 'paranoia', '~> 2.4'
gem 'tzinfo-data', platforms: %i[mingw mswin x64_mingw]
Expand Down
6 changes: 5 additions & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ GEM
railties (>= 3.2, < 5.2)
erubis (2.7.0)
execjs (2.7.0)
faker (1.7.3)
i18n (~> 0.5)
ffi (1.9.18)
font-awesome-rails (4.7.0.2)
railties (>= 3.2, < 5.2)
Expand Down Expand Up @@ -256,9 +258,11 @@ DEPENDENCIES
coveralls
devise (~> 3.0)
dotenv-rails
faker (~> 1.7.0)
geokit (~> 1.0)
haml (~> 5.0)
http_accept_language (~> 2.0)
kaminari (~> 1.0)
local_time (~> 2.0)
obscenity (~> 1.0, >= 1.0.2)
paranoia (~> 2.4)
Expand All @@ -281,4 +285,4 @@ RUBY VERSION
ruby 2.2.3p173

BUNDLED WITH
1.15.4
1.16.1
2 changes: 2 additions & 0 deletions app/assets/javascripts/adopted.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// Place all the behaviors and hooks related to the matching controller here.
// All this logic will automatically be available in application.js.
3 changes: 3 additions & 0 deletions app/assets/stylesheets/adopted.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// Place all the styles related to the Adopted controller here.
// They will automatically be included in application.css.
// You can use Sass (SCSS) here: http://sass-lang.com/
46 changes: 46 additions & 0 deletions app/controllers/adopted_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
class AdoptedController < ApplicationController
before_action :authenticate
before_action :get_adopted_things, :make_cur_page, :make_other_pages, only: [:index]
Copy link
Member Author

Choose a reason for hiding this comment

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

Given these filters are only used for the index action, what do you think of just moving the logic in there? You could still leave the helper functions and call them.


# GET /api/v1/drains/adopted
# Optional params:
#
# page
def index
@results = { next_page: @next_page, prev_page: @prev_page, total_pages: @adopted_things.page(1).total_pages, drains: @things }
render json: @results
end

private

def get_adopted_things
@adopted_things = Thing.where.not(user_id: nil)
Copy link
Member Author

Choose a reason for hiding this comment

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

There is already an adopted scope on Thing that you could use.

end

# Determine if the user supplied a valid page number, if not they get first page
def make_cur_page
page = ((params[:page].blank?) || (params[:page].to_i == 0)) ? 1 : params[:page]
@cur_page = @adopted_things.page(page)
Copy link
Member Author

Choose a reason for hiding this comment

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

This coupling feels a bit weird as it requires the developer to have called adopted_things first.

I think this can be simplified a bit if def index was like:

adopted_things = Things.adopted.order(:created_at).page(params[:page])

results = {
  next_page: adopted_things.next_page, # I think it's fine if this is null to the caller
  prev_page: adopted_things.prev_page,
  total_pages: adopted_things.total_pages,
  things: format_fields(things)
}

render json: @results

and then the make_cur_pages, make_other_pages and the shared state could be removed.

@things = format_fields(@cur_page)
end

# Determine next and previous pages, so the user can navigate if needed
def make_other_pages
@next_page = @cur_page.next_page.nil? ? -1 : @cur_page.next_page
@prev_page = @cur_page.prev_page.nil? ? -1 : @cur_page.prev_page
end

def format_fields(obj)
obj.map { |thing| {latitude: thing.lat, longitude: thing.lng, city_id: 'N-' + thing.city_id.to_s} }
end

def authenticate
authenticate_or_request_with_http_basic('Administration') do |username, password|
user = User.find_by(email: username)
if user && user.valid_password?(password)
return true if user.admin?
render html: '<div> You must be an admin to access this page </div>'.html_safe
end
end
end
end
2 changes: 2 additions & 0 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
require 'csv'

class ApplicationController < ActionController::Base
# Prevent CSRF attacks by raising an exception.
# For APIs, you may want to use :null_session instead.
Expand Down
2 changes: 2 additions & 0 deletions app/helpers/adopted_helper.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
module AdoptedHelper
end
5 changes: 5 additions & 0 deletions app/views/adopted/index.csv.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<%- headers = ['Lat', 'Lng', 'City ID'] -%>
Copy link
Member Author

Choose a reason for hiding this comment

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

Is this used anywhere? It didn't appear to be.

Copy link

Choose a reason for hiding this comment

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

No it's not. I removed CSV rendering, and the API allows for JSON only.

<%= CSV.generate_line headers %>
<%- @allthings.each do |thing| -%>
<%= CSV.generate_line([thing.lat, thing.lng, "N-" + thing.city_id.to_s]) -%>
<%- end -%>
8 changes: 7 additions & 1 deletion config/environments/development.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,13 @@
config.cache_classes = false

# asset host
config.action_controller.asset_host = 'http://' + Socket.ip_address_list[1].ip_address + ':3000'
# There are two cases for dev asset_host to be aware of:
#
# "localhost"/127.0.0.1 ... when developing locally
# remote domainname/IP ... when developing on a remote server
this_server = Socket.ip_address_list[1]
this_server_hostname = (this_server.getnameinfo[0] == 'localhost') ? 'localhost' : this_server.ip_address
config.action_controller.asset_host = 'http://' + this_server_hostname + ':3000'
config.action_mailer.asset_host = config.action_controller.asset_host

# Do not eager load code on boot.
Expand Down
2 changes: 1 addition & 1 deletion config/initializers/devise.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
# given strategies, for example, `config.http_authenticatable = [:database]` will
# enable it only for database authentication. The supported strategies are:
# :database = Support basic authentication with authentication key + password
# config.http_authenticatable = false
config.http_authenticatable = false

# If http headers should be returned for AJAX requests. True by default.
# config.http_authenticatable_on_xhr = true
Expand Down
10 changes: 10 additions & 0 deletions config/initializers/kaminari_config.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
Kaminari.configure do |config|
config.default_per_page = 100
config.max_per_page = 1000
# config.window = 4
# config.outer_window = 0
# config.left = 0
# config.right = 0
# config.page_method_name = :page
# config.param_name = :page
end
9 changes: 9 additions & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,13 @@
resource :things
mount RailsAdmin::Engine => '/admin', :as => 'rails_admin'
root to: 'main#index'

# API
scope '/api' do
scope '/v1' do
scope '/drains' do
get '/adopted' => 'adopted#index'
end
end
end
end
19 changes: 12 additions & 7 deletions db/seeds.rb
Original file line number Diff line number Diff line change
@@ -1,14 +1,8 @@
User.where(email: '[email protected]').first_or_initialize.tap do |user|
# Add a mock user with admin privileges
user.first_name = 'John'
user.last_name = 'Doe'
user.password = 'password'
user.save!
end

User.where(email: '[email protected]').first_or_initialize.tap do |user|
user.first_name = 'Jane'
user.last_name = 'Doe'
user.password = 'password'
user.admin = true
user.save!
end
Expand All @@ -24,3 +18,14 @@
thing.save!
end
end

1000.times do |i|
first_name = Faker::Name.first_name
last_name = Faker::Name.last_name
email = "user-#{i+1}@usertest.org"
password = "pass1234"
User.create!(first_name: first_name,
last_name: last_name,
email: email,
password: password)
end
17 changes: 17 additions & 0 deletions lib/tasks/data.rake
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,21 @@ namespace :data do
end
end
end

task auto_adopt: :environment do
# Make random users adopt drains to test server load when generating API data
# There has to be users in DB for this to work

if Rails.env.production?
puts "Can't run this in production"
else
Thing.first(1000).each_with_index do |t, i|
if t.user_id.blank?
t.user_id = User.order('RANDOM()').limit(1).first.id
t.save
end
puts i.to_s + " Adopting a drain"
end
end
end
end
65 changes: 65 additions & 0 deletions test/controllers/adopted_controller_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
require 'test_helper'

class AdoptedControllerTest < ActionController::TestCase
setup do
request.env['devise.mapping'] = Devise.mappings[:user]
@user = users(:erik)
@user2 = users(:dan)
@admin = users(:admin)
@thing = things(:thing_1)
@thing2 = things(:thing_2)

@thing.user_id = @user.id
@thing2.user_id = @user2.id
@thing.save
@thing2.save
end

test 'should get index' do
@request.env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials(@user.email, 'correct')

get :index
assert_response :success
end

test 'should get json' do
@request.env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials(@admin.email, 'correct')

get :index
assert_equal 'application/json', @response.content_type
end

test 'only admins get access' do
@request.env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials(@user.email, 'correct')

get :index
assert_equal 'text/html', @response.content_type # If user were an admin, content_type would be JSON, since that is default
end


test 'drain data is correct' do
@request.env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials(@admin.email, 'correct')

get :index
random_drain = JSON.parse(@response.body)["drains"].first

drain = Thing.find_by(city_id: random_drain["city_id"].gsub("N-",""))

assert_not_nil drain
assert_equal drain.lat.to_s, random_drain["latitude"]
assert_equal drain.lng.to_s, random_drain["longitude"]

end

test 'page counts' do
@request.env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials(@admin.email, 'correct')

get :index
json = JSON.parse(@response.body)
puts(json)

assert_equal json["next_page"], 2
assert_equal json["prev_page"], -1
assert_equal json["total_pages"], 1
end
end