-
Notifications
You must be signed in to change notification settings - Fork 56
236 simple api #241
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
base: main
Are you sure you want to change the base?
236 simple api #241
Changes from 19 commits
1ad6247
900eae4
43f7697
3b65084
69d6af7
b1b4305
91b0e3b
7371587
15fae71
7287643
160a045
0b4dab3
ceca853
b8fff22
88a33a7
b893346
973efac
5f868d1
5772058
da2f4b1
01c0719
343a600
78c0c75
cfeba19
417c95f
2953cf3
e24cea2
dcc6277
e66a624
4032d8d
c375680
ea959a8
57f6f42
739d86c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ ruby '2.2.3' | |
|
|
||
| gem 'dotenv-rails', groups: [:development, :test] | ||
| gem 'rails', '~> 4.2.4' | ||
| gem 'faker', '1.7.3' | ||
| gem 'airbrake', '~> 5.2' | ||
| gem 'devise' | ||
| gem 'geokit' | ||
|
|
@@ -25,6 +26,7 @@ end | |
| group :development do | ||
| gem 'byebug' | ||
| gem 'spring' | ||
| gem 'kaminari' | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We'll need this gem in production too, no? It feels like it should be at the top level. Do you mind locking it to a major version as well (I locked the rest in #259) so that it is easier to update without breaking compatibility.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ~>1.0 includes supported versions. the older version dropped support for an earlier version of Ruby |
||
| end | ||
|
|
||
| group :production do | ||
|
|
||
| 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. |
| 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/ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| class AdoptedController < ApplicationController | ||
| before_action :authenticate | ||
| before_action :make_cur_page, :make_other_pages, only: [:index] | ||
|
|
||
| # GET /api/v1/drains/adopted | ||
| # Optional params: | ||
| # | ||
| # format=[json|xml|csv] | ||
| # page | ||
| def index | ||
| @results = {next_page: @next_page, prev_page: @prev_page, drains: @things} | ||
| render_types | ||
| end | ||
|
|
||
| private | ||
|
|
||
| # Determine if the user supplied a valid page number, if not they get first page | ||
| def make_cur_page | ||
| page = params[:page].blank? ? 2 : params[:page] | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be |
||
| @cur_page = Thing.where.not(user_id: nil).page(page) | ||
| @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? ? 0 : @cur_page.next_page | ||
| @prev_page = @cur_page.prev_page.nil? ? 0 : @cur_page.prev_page | ||
| end | ||
|
|
||
| def render_types | ||
| respond_to do |format| | ||
| format.csv do | ||
| headers['Content-Type'] ||= 'text/csv' | ||
| headers['Content-Disposition'] = 'attachment; filename=\'adopted_drains.csv\'' | ||
|
|
||
| # Assuming that a CSV was requested in order to retrieve all results at once | ||
| @allthings = Thing.where.not(user_id: nil) | ||
| end | ||
| format.xml { render xml: @results } | ||
| format.all { render json: @results } | ||
| end | ||
| 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 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| module AdoptedHelper | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| <%- headers = ['Lat', 'Lng', 'City ID'] -%> | ||
| <%= CSV.generate_line headers %> | ||
| <%- @allthings.each do |thing| -%> | ||
| <%= CSV.generate_line([thing.lat, thing.lng, "N-" + thing.city_id.to_s]) -%> | ||
| <%- end -%> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why was this uncommented?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Never mind, I see that the API is locked to authenticated users.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cool |
||
|
|
||
| # If http headers should be returned for AJAX requests. True by default. | ||
| # config.http_authenticatable_on_xhr = true | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| Kaminari.configure do |config| | ||
| config.default_per_page = 1000 | ||
| # config.max_per_page = nil | ||
| # config.window = 4 | ||
| # config.outer_window = 0 | ||
| # config.left = 0 | ||
| # config.right = 0 | ||
| # config.page_method_name = :page | ||
| # config.param_name = :page | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,4 +19,13 @@ | |
| resource :things | ||
| mount RailsAdmin::Engine => '/admin', :as => 'rails_admin' | ||
| root to: 'main#index' | ||
|
|
||
| # API | ||
| scope '/api' do | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 to the URL pathing here |
||
| scope '/v1' do | ||
| scope '/drains' do | ||
| get '/adopted' => 'adopted#index' | ||
| end | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |
|
|
||
| r = Random.new | ||
|
|
||
| =begin | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you mean to leave this commented? |
||
| 500.times do |i| | ||
| Thing.where(city_id: i).first_or_initialize.tap do |thing| | ||
| thing.name = "Some Drain #{i}" | ||
|
|
@@ -15,3 +16,18 @@ | |
| thing.save! | ||
| end | ||
| 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 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,4 +6,18 @@ namespace :data do | |
|
|
||
| ThingImporter.load('https://data.sfgov.org/api/views/jtgq-b7c5/rows.csv?accessType=DOWNLOAD') | ||
| 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 | ||
|
|
||
| unless Rails.env.production? | ||
| Thing.first(10_000).each do |t| | ||
| if t.user_id.blank? | ||
| t.user_id = User.find_by('id' => Random.rand(1..User.last.id)).id | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think something like |
||
| t.save | ||
| end | ||
| end | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| require 'test_helper' | ||
|
|
||
| class AdoptedControllerTest < ActionController::TestCase | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like checking the content-type, but could we make a few assertions on the response content to make sure the right records are returned? It'd also be nice to exercise the paging logic a little here. |
||
| setup do | ||
| request.env['devise.mapping'] = Devise.mappings[:user] | ||
| @user = users(:erik) | ||
| @admin = users(:admin) | ||
| 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, format: :json | ||
| assert_equal 'application/json', @response.content_type | ||
| end | ||
|
|
||
| test 'should get xml' do | ||
| @request.env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials(@admin.email, 'correct') | ||
|
|
||
| get :index, format: :xml | ||
| assert_equal 'application/xml', @response.content_type | ||
| end | ||
|
|
||
| test 'should get csv' do | ||
| @request.env['HTTP_AUTHORIZATION'] = ActionController::HttpAuthentication::Basic.encode_credentials(@admin.email, 'correct') | ||
|
|
||
| get :index, format: :csv | ||
| assert_equal 'text/csv', @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 | ||
| end | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need an exact version here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed that to have the version ~> 1.7.0... These are the sure compatible versions, not sure if they will change support in the future.