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 all 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 @@ -3,9 +3,11 @@ ruby '2.2.3'

gem 'airbrake', '~> 7.1'
gem 'devise', '~> 3.0'
gem 'faker', '~> 1.7.0'
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 needed for all environments? It feels like it is only needed for test and maybe development.

gem 'geokit', '~> 1.0'
gem 'haml', '~> 5.0'
gem 'http_accept_language', '~> 2.0'
gem 'kaminari', '~> 1.0'
gem 'local_time', '~> 2.0'
gem 'obscenity', '~> 1.0', '>= 1.0.2'
gem 'pg'
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/
48 changes: 48 additions & 0 deletions app/controllers/adopted_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
class AdoptedController < ApplicationController
before_action :authenticate

# GET /api/v1/drains/adopted
# Optional params:
#
# page
def index
adopted_things
make_cur_page
make_other_pages
@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 adopted_things
@adopted_things = Thing.adopted
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.zero? ? 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>'
Copy link
Member Author

Choose a reason for hiding this comment

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

Feels like we should return a 401 here in addition to the rendered HTML.

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
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
20 changes: 18 additions & 2 deletions lib/tasks/data.rake
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,7 @@ namespace :data do
ENV['MAXIMUM_MOVEMENT_IN_FEET'] || raise('$MAXIMUM_MOVEMENT_IN_FEET required')

adoption_deletion_from = Time.zone.parse(ENV['ADOPTION_DELETION_FROM'])

moved_adoptions = AdoptionMover.move_close_deleted_adoptions(adoption_deletion_from, ENV['MAXIMUM_MOVEMENT_IN_FEET'])

CSV($stdout) do |csv|
csv << %w[from to]
moved_adoptions.each do |from, to|
Expand All @@ -28,3 +26,21 @@ namespace :data do
end
end
end

namespace :modify do
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
62 changes: 62 additions & 0 deletions test/controllers/adopted_controller_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
require 'test_helper'
require 'rake'

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
Rails.application.load_seed # Seed the user with users and drains
Rake::Task['modify:auto_adopt'].invoke # Adopt the seeded drains with seeded users
@request.env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials(@admin.email, 'correct')
get :index
json = JSON.parse(@response.body)

assert_equal json['next_page'], 2
assert_equal json['prev_page'], -1
assert_equal json['total_pages'], 5 # Should be 5 - default drains per page is 100 and we seeded the DB with 500
end
end