Added better messages and error handling in a couple of places. Added integration...

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 012ba2446787d5d313c983f59371ba5850789386..c701d8adde71093f37b9f927c1e8b228e4d5bbc6 100644 (file)
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -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
 

diff --git a/app/controllers/user_blocks_controller.rb b/app/controllers/user_blocks_controller.rb
index 7d3830c251791c029b737fb5a2885da4b73bc418..f6bca4bced9f870c89b7db7bb3b19bd2bbd1114a 100644 (file)
--- a/app/controllers/user_blocks_controller.rb
+++ b/app/controllers/user_blocks_controller.rb
@@ -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
 
   ##

diff --git a/app/models/user.rb b/app/models/user.rb
index 5268446724c9dd587d5c0e46ecbf883191792188..95f0e39861906632d208883080f93d8c5f851cc7 100644 (file)
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -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

diff --git a/app/views/user_blocks/not_found.html.erb b/app/views/user_blocks/not_found.html.erb
new file mode 100644 (file)
index 0000000..3b5323d
--- /dev/null
+++ b/app/views/user_blocks/not_found.html.erb
@@ -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 %>

diff --git a/config/locales/en.yml b/config/locales/en.yml
index 363f620f4c6f4a63f93135b0e7e5846ef5d1d32b..241e0327f12c84c1f5bdddd32e4a53ec627bd9c3 100644 (file)
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -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."

diff --git a/test/integration/user_blocks_test.rb b/test/integration/user_blocks_test.rb
new file mode 100644 (file)
index 0000000..822c923
--- /dev/null
+++ b/test/integration/user_blocks_test.rb
@@ -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