Added better messages and error handling in a couple of places. Added integration checks to ensure that the blocking is actually working. Tests FTW.

This commit is contained in:
Matt Amos 2009-09-30 17:39:42 +00:00
parent 95c3d640a4
commit 77851bac7b
6 changed files with 73 additions and 4 deletions

View file

@ -80,9 +80,9 @@ class ApplicationController < ActionController::Base
end
# check if the user has been banned
unless @user.nil? or @user.blocks.empty?
unless @user.nil? or @user.active_blocks.empty?
# NOTE: need slightly more helpful message than this.
render :text => "You got banned!", :status => :forbidden
render :text => t('application.setup_user_auth.blocked'), :status => :forbidden
end
end

View file

@ -122,12 +122,16 @@ class UserBlocksController < ApplicationController
# ensure that there is a "this_user" instance variable
def lookup_this_user
@this_user = User.find_by_display_name(params[:display_name])
rescue ActiveRecord::RecordNotFound
redirect_to :controller => 'user', :action => 'view', :display_name => params[:display_name] unless @this_user
end
##
# ensure that there is a "user_block" instance variable
def lookup_user_block
@user_block = UserBlock.find(params[:id])
rescue ActiveRecord::RecordNotFound
render :action => "not_found", :status => :not_found
end
##

View file

@ -14,7 +14,7 @@ class User < ActiveRecord::Base
has_many :client_applications
has_many :oauth_tokens, :class_name => "OauthToken", :order => "authorized_at desc", :include => [:client_application]
has_many :blocks, :class_name => "UserBlock", :conditions => ["user_blocks.ends_at > now() or user_blocks.needs_view"]
has_many :active_blocks, :class_name => "UserBlock", :conditions => ['user_blocks.ends_at > \'#{Time.now.getutc.xmlschema(5)}\' or user_blocks.needs_view']
has_many :roles, :class_name => "UserRole"
validates_presence_of :email, :display_name
@ -150,7 +150,7 @@ class User < ActiveRecord::Base
# returns the first active block which would require users to view
# a message, or nil if there are none.
def blocked_on_view
blocks.inject(nil) { |s,x| s || (x.needs_view? ? x : nil) }
active_blocks.inject(nil) { |s,x| s || (x.needs_view? ? x : nil) }
end
def delete

View file

@ -0,0 +1,3 @@
<p><%= t'user_block.not_found.sorry', :id => params[:id] %></p>
<%= link_to t('user_block.not_found.back'), user_blocks_path %>

View file

@ -800,6 +800,9 @@ en:
scheduled_for_deletion: "Track scheduled for deletion"
make_public:
made_public: "Track made public"
application:
setup_user_auth:
blocked: "Your access to the API has been blocked. Please log-in to the web interface to find out more."
oauth:
oauthorize:
request_access: "The application {{app_name}} is requesting access to your account. Please check whether you would like the application to have the following capabilities. You may choose as many or as few as you like."
@ -1012,6 +1015,9 @@ en:
model:
non_moderator_update: "Must be a moderator to create or update a block."
non_moderator_revoke: "Must be a moderator to revoke a block."
not_found:
sorry: "Sorry, the user block with ID {{id}} could not be found."
back: "Back to index"
new:
reason: "The reason why {{name}} is being blocked. Please be as calm and as reasonable as possible, giving as much detail as you can about the situation, remembering that the message will be publicly visible. Bear in mind that not all users understand the community jargon, so please try to use laymans terms."
period: "How long, starting now, the user will be blocked from the API for."

View file

@ -0,0 +1,56 @@
require File.dirname(__FILE__) + '/../test_helper'
class UserBlocksTest < ActionController::IntegrationTest
fixtures :users, :user_blocks
def auth_header(user, pass)
{"HTTP_AUTHORIZATION" => "Basic %s" % Base64.encode64("#{user}:#{pass}")}
end
def test_api_blocked
blocked_user = users(:public_user)
get "/api/#{API_VERSION}/user/details"
assert_response :unauthorized
get "/api/#{API_VERSION}/user/details", nil, auth_header(blocked_user.display_name, "test")
assert_response :success
# now block the user
UserBlock.create(:user_id => blocked_user.id,
:creator_id => users(:moderator_user).id,
:reason => "testing",
:ends_at => Time.now.getutc + 5.minutes)
get "/api/#{API_VERSION}/user/details", nil, auth_header(blocked_user.display_name, "test")
assert_response :forbidden
end
def test_api_revoke
blocked_user = users(:public_user)
moderator = users(:moderator_user)
block = UserBlock.create(:user_id => blocked_user.id,
:creator_id => moderator.id,
:reason => "testing",
:ends_at => Time.now.getutc + 5.minutes)
get "/api/#{API_VERSION}/user/details", nil, auth_header(blocked_user.display_name, "test")
assert_response :forbidden
# revoke the ban
post '/login', {'user[email]' => moderator.email, 'user[password]' => "test", :referer => "/blocks/#{block.id}/revoke"}
assert_response :redirect
follow_redirect!
assert_response :success
assert_template 'user_blocks/revoke'
post "/blocks/#{block.id}/revoke", {'confirm' => "yes"}
assert_response :redirect
follow_redirect!
assert_response :success
assert_template 'user_blocks/show'
reset!
# access the API again. this time it should work
get "/api/#{API_VERSION}/user/details", nil, auth_header(blocked_user.display_name, "test")
assert_response :success
end
end