fix spec, bugs, style

This commit is contained in:
Devin Howard 2016-12-12 22:28:10 -05:00
parent 87228c27c1
commit 186129807e
22 changed files with 54 additions and 64 deletions

View file

@ -6,7 +6,6 @@ class AccessController < ApplicationController
:deny_access, :deny_access_post, :request_access] :deny_access, :deny_access_post, :request_access]
after_action :verify_authorized after_action :verify_authorized
# GET maps/:id/request_access # GET maps/:id/request_access
def request_access def request_access
@map = nil @map = nil

View file

@ -22,7 +22,7 @@ class ApplicationController < ActionController::Base
helper_method :admin? helper_method :admin?
def handle_unauthorized def handle_unauthorized
if authenticated? and params[:controller] == 'maps' and params[:action] == 'show' if authenticated? && (params[:controller] == 'maps') && (params[:action] == 'show')
redirect_to request_access_map_path(params[:id]) redirect_to request_access_map_path(params[:id])
elsif authenticated? elsif authenticated?
redirect_to root_path, notice: "You don't have permission to see that page." redirect_to root_path, notice: "You don't have permission to see that page."
@ -41,13 +41,13 @@ class ApplicationController < ActionController::Base
def require_no_user def require_no_user
return true unless authenticated? return true unless authenticated?
redirect_to edit_user_path(user), notice: 'You must be logged out.' redirect_to edit_user_path(user), notice: 'You must be logged out.'
return false false
end end
def require_user def require_user
return true if authenticated? return true if authenticated?
redirect_to sign_in_path, notice: 'You must be logged in.' redirect_to sign_in_path, notice: 'You must be logged in.'
return false false
end end
def require_admin def require_admin

View file

@ -14,13 +14,13 @@ class TopicsController < ApplicationController
@topics = policy_scope(Topic).where('LOWER("name") like ?', term.downcase + '%').order('"name"') @topics = policy_scope(Topic).where('LOWER("name") like ?', term.downcase + '%').order('"name"')
@mapTopics = @topics.select { |t| t&.metacode&.name == 'Metamap' } @mapTopics = @topics.select { |t| t&.metacode&.name == 'Metamap' }
# prioritize topics which point to maps, over maps # prioritize topics which point to maps, over maps
@exclude = @mapTopics.length > 0 ? @mapTopics.map(&:name) : [''] @exclude = @mapTopics.length.positive? ? @mapTopics.map(&:name) : ['']
@maps = policy_scope(Map).where('LOWER("name") like ? AND name NOT IN (?)', term.downcase + '%', @exclude).order('"name"') @maps = policy_scope(Map).where('LOWER("name") like ? AND name NOT IN (?)', term.downcase + '%', @exclude).order('"name"')
else else
@topics = [] @topics = []
@maps = [] @maps = []
end end
@all= @topics.to_a.concat(@maps.to_a).sort { |a, b| a.name <=> b.name } @all = @topics.to_a.concat(@maps.to_a).sort_by(&:name)
render json: autocomplete_array_json(@all).to_json render json: autocomplete_array_json(@all).to_json
end end

View file

@ -21,13 +21,10 @@ class Users::RegistrationsController < Devise::RegistrationsController
end end
end end
private private
def store_location def store_location
if params[:redirect_to] store_location_for(User, params[:redirect_to]) if params[:redirect_to]
store_location_for(User, params[:redirect_to])
end
end end
def configure_sign_up_params def configure_sign_up_params

View file

@ -20,7 +20,7 @@ module TopicsHelper
type: is_map ? metamapMetacode.name : t.metacode.name, type: is_map ? metamapMetacode.name : t.metacode.name,
typeImageURL: is_map ? metamapMetacode.icon : t.metacode.icon, typeImageURL: is_map ? metamapMetacode.icon : t.metacode.icon,
mapCount: is_map ? 0 : t.maps.count, mapCount: is_map ? 0 : t.maps.count,
synapseCount: is_map ? 0 : t.synapses.count, synapseCount: is_map ? 0 : t.synapses.count
} }
end end
end end

View file

@ -1,3 +1,4 @@
# frozen_string_literal: true
class AccessRequest < ApplicationRecord class AccessRequest < ApplicationRecord
belongs_to :user belongs_to :user
belongs_to :map belongs_to :map
@ -5,7 +6,7 @@ class AccessRequest < ApplicationRecord
def approve def approve
self.approved = true self.approved = true
self.answered = true self.answered = true
self.save save
Mailboxer::Notification.where(notified_object: self).find_each do |notification| Mailboxer::Notification.where(notified_object: self).find_each do |notification|
Mailboxer::Receipt.where(notification: notification).update_all(is_read: true) Mailboxer::Receipt.where(notification: notification).update_all(is_read: true)
@ -18,7 +19,7 @@ class AccessRequest < ApplicationRecord
def deny def deny
self.approved = false self.approved = false
self.answered = true self.answered = true
self.save save
Mailboxer::Notification.where(notified_object: self).find_each do |notification| Mailboxer::Notification.where(notified_object: self).find_each do |notification|
Mailboxer::Receipt.where(notification: notification).update_all(is_read: true) Mailboxer::Receipt.where(notification: notification).update_all(is_read: true)
@ -26,10 +27,10 @@ class AccessRequest < ApplicationRecord
end end
def requested_text def requested_text
self.map.name + ' - request to edit' map.name + ' - request to edit'
end end
def approved_text def approved_text
self.map.name + ' - access approved' map.name + ' - access approved'
end end
end end

View file

@ -125,6 +125,6 @@ class Map < ApplicationRecord
end end
def invited_text def invited_text
self.name + ' - invited to edit' name + ' - invited to edit'
end end
end end

View file

@ -14,9 +14,7 @@ Webhooks::Slack::Base = Struct.new(:webhook, :event) do
'something' 'something'
end end
def channel delegate :channel, to: :webhook
webhook.channel
end
def attachments def attachments
[{ [{

View file

@ -1,3 +1,4 @@
# frozen_string_literal: true
class ExplorePolicy < ApplicationPolicy class ExplorePolicy < ApplicationPolicy
def active? def active?
true true

View file

@ -1,3 +1,4 @@
# frozen_string_literal: true
class HackPolicy < ApplicationPolicy class HackPolicy < ApplicationPolicy
def load_url_title? def load_url_title?
true true

View file

@ -16,7 +16,7 @@ class MapPolicy < ApplicationPolicy
end end
def show? def show?
record.permission.in?(['commons', 'public']) || record.permission.in?(%w(commons public)) ||
record.collaborators.include?(user) || record.collaborators.include?(user) ||
record.user == user record.user == user
end end

View file

@ -22,7 +22,7 @@ class TopicPolicy < ApplicationPolicy
if record.defer_to_map.present? if record.defer_to_map.present?
map_policy.show? map_policy.show?
else else
record.permission.in?(['commons', 'public']) || record.user == user record.permission.in?(%w(commons public)) || record.user == user
end end
end end

View file

@ -1,6 +1,5 @@
# frozen_string_literal: true # frozen_string_literal: true
class NotificationService class NotificationService
def self.renderer def self.renderer
renderer ||= ApplicationController.renderer.new( renderer ||= ApplicationController.renderer.new(
http_host: ENV['MAILER_DEFAULT_URL'], http_host: ENV['MAILER_DEFAULT_URL'],

View file

@ -9,20 +9,20 @@ Doorkeeper.configure do
current_user current_user
else else
store_location_for(User, request.fullpath) store_location_for(User, request.fullpath)
redirect_to(sign_in_url, notice: "Sign In to Connect") redirect_to(sign_in_url, notice: 'Sign In to Connect')
end end
end end
# If you want to restrict access to the web interface for adding oauth authorized applications, # If you want to restrict access to the web interface for adding oauth authorized applications,
# you need to declare the block below. # you need to declare the block below.
admin_authenticator do admin_authenticator do
if current_user && current_user.admin if current_user&.admin
current_user current_user
elsif current_user && !current_user.admin elsif current_user && !current_user.admin
redirect_to(root_url, notice: "Unauthorized") redirect_to(root_url, notice: 'Unauthorized')
else else
store_location_for(User, request.fullpath) store_location_for(User, request.fullpath)
redirect_to(sign_in_url, notice: "Try signing in to do that") redirect_to(sign_in_url, notice: 'Try signing in to do that')
end end
end end

View file

@ -1,3 +1,4 @@
# frozen_string_literal: true
class Rack::Attack class Rack::Attack
Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new
@ -11,10 +12,8 @@ class Rack::Attack
# Throttle POST requests to /login by IP address # Throttle POST requests to /login by IP address
# #
# Key: "rack::attack:#{Time.now.to_i/:period}:logins/ip:#{req.ip}" # Key: "rack::attack:#{Time.now.to_i/:period}:logins/ip:#{req.ip}"
throttle('logins/ip', :limit => 5, :period => 20.seconds) do |req| throttle('logins/ip', limit: 5, period: 20.seconds) do |req|
if req.path == '/login' && req.post? req.ip if req.path == '/login' && req.post?
req.ip
end
end end
# Throttle POST requests to /login by email param # Throttle POST requests to /login by email param
@ -25,17 +24,17 @@ class Rack::Attack
# throttle logins for another user and force their login requests to be # throttle logins for another user and force their login requests to be
# denied, but that's not very common and shouldn't happen to you. (Knock # denied, but that's not very common and shouldn't happen to you. (Knock
# on wood!) # on wood!)
throttle("logins/email", :limit => 5, :period => 20.seconds) do |req| throttle('logins/email', limit: 5, period: 20.seconds) do |req|
if req.path == '/login' && req.post? if req.path == '/login' && req.post?
# return the email if present, nil otherwise # return the email if present, nil otherwise
req.params['email'].presence req.params['email'].presence
end end
end end
throttle('load_url_title/req/5mins/ip', :limit => 300, :period => 5.minutes) do |req| throttle('load_url_title/req/5mins/ip', limit: 300, period: 5.minutes) do |req|
req.ip if req.path == 'hacks/load_url_title' req.ip if req.path == 'hacks/load_url_title'
end end
throttle('load_url_title/req/1s/ip', :limit => 5, :period => 1.second) do |req| throttle('load_url_title/req/1s/ip', limit: 5, period: 1.second) do |req|
# If the return value is truthy, the cache key for the return value # If the return value is truthy, the cache key for the return value
# is incremented and compared with the limit. In this case: # is incremented and compared with the limit. In this case:
# "rack::attack:#{Time.now.to_i/1.second}:load_url_title/req/ip:#{req.ip}" # "rack::attack:#{Time.now.to_i/1.second}:load_url_title/req/ip:#{req.ip}"

View file

@ -1,8 +1,4 @@
# Sample localization file for English. Add more files in this directory for other locales.
# See https://github.com/svenfuchs/rails-i18n/tree/master/rails%2Flocale for starting points.
en: en:
activerecord: mailboxer:
attributes: notification_mailer:
user: subject: "%{subject}"
joinedwithcode: "Access code"

View file

@ -18,7 +18,6 @@ RSpec.describe 'mappings API', type: :request do
it 'GET /api/v2/mappings/:id' do it 'GET /api/v2/mappings/:id' do
get "/api/v2/mappings/#{mapping.id}", params: { access_token: token } get "/api/v2/mappings/#{mapping.id}", params: { access_token: token }
expect(response).to have_http_status(:success) expect(response).to have_http_status(:success)
expect(response).to match_json_schema(:mapping) expect(response).to match_json_schema(:mapping)
expect(JSON.parse(response.body)['data']['id']).to eq mapping.id expect(JSON.parse(response.body)['data']['id']).to eq mapping.id

View file

@ -1,3 +1,4 @@
# frozen_string_literal: true
# t frozen_string_literal: true # t frozen_string_literal: true
require 'rails_helper' require 'rails_helper'

View file

@ -18,7 +18,6 @@ RSpec.describe 'topics API', type: :request do
it 'GET /api/v2/topics/:id' do it 'GET /api/v2/topics/:id' do
get "/api/v2/topics/#{topic.id}" get "/api/v2/topics/#{topic.id}"
expect(response).to have_http_status(:success) expect(response).to have_http_status(:success)
expect(response).to match_json_schema(:topic) expect(response).to match_json_schema(:topic)
expect(JSON.parse(response.body)['data']['id']).to eq topic.id expect(JSON.parse(response.body)['data']['id']).to eq topic.id

View file

@ -53,9 +53,9 @@ RSpec.describe SynapsesController, type: :controller do
expect(response.status).to eq 422 expect(response.status).to eq 422
end end
it 'does not create a synapse' do it 'does not create a synapse' do
expect { expect do
post :create, format: :json, params: { synapse: invalid_attributes } post :create, format: :json, params: { synapse: invalid_attributes }
}.to change { end.to change {
Synapse.count Synapse.count
}.by 0 }.by 0
end end

View file

@ -1,10 +1,11 @@
# frozen_string_literal: true
require 'rails_helper' require 'rails_helper'
RSpec.describe MapMailer, type: :mailer do RSpec.describe MapMailer, type: :mailer do
describe 'access_request_email' do describe 'access_request_email' do
let(:request) { create(:access_request) }
let(:map) { create(:map) } let(:map) { create(:map) }
let(:mail) { described_class.access_request_email(request, map) } let(:request) { create(:access_request, map: map) }
let(:mail) { described_class.access_request_email(request) }
it { expect(mail.from).to eq ['team@metamaps.cc'] } it { expect(mail.from).to eq ['team@metamaps.cc'] }
it { expect(mail.to).to eq [map.user.email] } it { expect(mail.to).to eq [map.user.email] }

View file

@ -1,8 +1,7 @@
# frozen_string_literal: true
require 'rails_helper' require 'rails_helper'
RSpec.describe AccessRequest, type: :model do RSpec.describe AccessRequest, type: :model do
include ActiveJob::TestHelper # enqueued_jobs
let(:access_request) { create(:access_request) } let(:access_request) { create(:access_request) }
describe 'approve' do describe 'approve' do
@ -13,7 +12,7 @@ RSpec.describe AccessRequest, type: :model do
it { expect(access_request.approved).to be true } it { expect(access_request.approved).to be true }
it { expect(access_request.answered).to be true } it { expect(access_request.answered).to be true }
it { expect(UserMap.count).to eq 1 } it { expect(UserMap.count).to eq 1 }
it { expect(enqueued_jobs.count).to eq 1 } it { expect(Mailboxer::Notification.count).to eq 1 }
end end
describe 'deny' do describe 'deny' do
@ -24,6 +23,6 @@ RSpec.describe AccessRequest, type: :model do
it { expect(access_request.approved).to be false } it { expect(access_request.approved).to be false }
it { expect(access_request.answered).to be true } it { expect(access_request.answered).to be true }
it { expect(UserMap.count).to eq 0 } it { expect(UserMap.count).to eq 0 }
it { expect(enqueued_jobs.count).to eq 0 } it { expect(Mailboxer::Notification.count).to eq 0 }
end end
end end