Improve blocks on/by links on user pages

All block related links are now only shown if the user has given
or received any blocks, and include a count of active blocks.

diff --git a/app/controllers/amf_controller.rb b/app/controllers/amf_controller.rb
index a24ac52651dec7e63f95f65c4d6f88dd15fd41b2..ef7a636390de6df9ecea484bd6331ea91af66749 100644 (file)
--- a/app/controllers/amf_controller.rb
+++ b/app/controllers/amf_controller.rb
@@ -133,7 +133,7 @@ class AmfController < ApplicationController
     amf_handle_error("'startchangeset'",nil,nil) do
       user = getuser(usertoken)
       if !user then return -1,"You are not logged in, so Potlatch can't write any changes to the database." end
-      unless user.active_blocks.empty? then return -1,t('application.setup_user_auth.blocked') end
+      if user.blocks.active.exists? then return -1,t('application.setup_user_auth.blocked') end
       if REQUIRE_TERMS_AGREED and user.terms_agreed.nil? then return -1,"You must accept the contributor terms before you can edit." end
 
       if cstags
@@ -459,7 +459,7 @@ class AmfController < ApplicationController
     amf_handle_error_with_timeout("'findgpx'" ,nil,nil) do
       user = getuser(usertoken)
       if !user then return -1,"You must be logged in to search for GPX traces." end
-      unless user.active_blocks.empty? then return -1,t('application.setup_user_auth.blocked') end
+      if user.blocks.active.exists? then return -1,t('application.setup_user_auth.blocked') end
 
       query = Trace.visible_to(user)
       if searchterm.to_i > 0 then
@@ -523,7 +523,7 @@ class AmfController < ApplicationController
     amf_handle_error("'putrelation' #{relid}" ,'relation',relid)  do
       user = getuser(usertoken)
       if !user then return -1,"You are not logged in, so the relation could not be saved." end
-      unless user.active_blocks.empty? then return -1,t('application.setup_user_auth.blocked') end
+      if user.blocks.active.exists? then return -1,t('application.setup_user_auth.blocked') end
       if REQUIRE_TERMS_AGREED and user.terms_agreed.nil? then return -1,"You must accept the contributor terms before you can edit." end
 
       if !tags_ok(tags) then return -1,"One of the tags is invalid. Linux users may need to upgrade to Flash Player 10.1." end
@@ -613,7 +613,7 @@ class AmfController < ApplicationController
   
       user = getuser(usertoken)
       if !user then return -1,"You are not logged in, so the way could not be saved." end
-      unless user.active_blocks.empty? then return -1,t('application.setup_user_auth.blocked') end
+      if user.blocks.active.exists? then return -1,t('application.setup_user_auth.blocked') end
       if REQUIRE_TERMS_AGREED and user.terms_agreed.nil? then return -1,"You must accept the contributor terms before you can edit." end
 
       if pointlist.length < 2 then return -2,"Server error - way is only #{points.length} points long." end
@@ -722,7 +722,7 @@ class AmfController < ApplicationController
     amf_handle_error("'putpoi' #{id}", 'node',id) do
       user = getuser(usertoken)
       if !user then return -1,"You are not logged in, so the point could not be saved." end
-      unless user.active_blocks.empty? then return -1,t('application.setup_user_auth.blocked') end
+      if user.blocks.active.exists? then return -1,t('application.setup_user_auth.blocked') end
       if REQUIRE_TERMS_AGREED and user.terms_agreed.nil? then return -1,"You must accept the contributor terms before you can edit." end
 
       if !tags_ok(tags) then return -1,"One of the tags is invalid. Linux users may need to upgrade to Flash Player 10.1." end
@@ -807,7 +807,7 @@ class AmfController < ApplicationController
     amf_handle_error("'deleteway' #{way_id}" ,'way', way_id) do
       user = getuser(usertoken)
       unless user then return -1,"You are not logged in, so the way could not be deleted." end
-      unless user.active_blocks.empty? then return -1,t('application.setup_user_auth.blocked') end
+      if user.blocks.active.exists? then return -1,t('application.setup_user_auth.blocked') end
       if REQUIRE_TERMS_AGREED and user.terms_agreed.nil? then return -1,"You must accept the contributor terms before you can edit." end
       
       way_id = way_id.to_i

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index b512a404dc5a3c7604cd9dffbe4b8018e23f1771..8f9ae29481b8baeee5517deb1c5e80c6670c9eec 100644 (file)
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -146,7 +146,7 @@ class ApplicationController < ActionController::Base
     # have we identified the user?
     if @user
       # check if the user has been banned
-      if not  @user.active_blocks.empty?
+      if @user.blocks.active.exists?
         # NOTE: need slightly more helpful message than this.
         report_error t('application.setup_user_auth.blocked'), :forbidden
       end

diff --git a/app/models/user.rb b/app/models/user.rb
index 1b9dd54e768e043a773e12615136fbb49138cfd6..68537e749fa5c794bba5ef5f13a49911f563e829 100644 (file)
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -16,7 +16,10 @@ class User < ActiveRecord::Base
   has_many :client_applications
   has_many :oauth_tokens, :class_name => "OauthToken", :order => "authorized_at desc", :include => [:client_application]
 
-  has_many :active_blocks, :class_name => "UserBlock", :conditions => proc { [ "user_blocks.ends_at > :ends_at or user_blocks.needs_view", { :ends_at => Time.now.getutc } ] }
+  has_many :blocks, :class_name => "UserBlock"
+  has_many :blocks_created, :class_name => "UserBlock", :foreign_key => :creator_id
+  has_many :blocks_revoked, :class_name => "UserBlock", :foreign_key => :revoker_id
+
   has_many :roles, :class_name => "UserRole"
 
   scope :visible, where(:status => ["pending", "active", "confirmed"])
@@ -182,7 +185,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
-    active_blocks.detect { |b| b.needs_view? }
+    blocks.active.detect { |b| b.needs_view? }
   end
 
   ##

diff --git a/app/models/user_block.rb b/app/models/user_block.rb
index 8821926bbd3b73a8cb37af6c09dbdd0722326239..2cf0eefc419fd3770c2c339819ac1beb56d1f169 100644 (file)
--- a/app/models/user_block.rb
+++ b/app/models/user_block.rb
@@ -9,6 +9,12 @@ class UserBlock < ActiveRecord::Base
 
   PERIODS = USER_BLOCK_PERIODS
 
+  ##
+  # scope to match active blocks
+  def self.active
+    self.where("needs_view or ends_at > ?", Time.now.getutc)
+  end
+
   ##
   # return a renderable version of the reason text.
   def reason

diff --git a/app/views/user/view.html.erb b/app/views/user/view.html.erb
index 95b66e864856c53f63b1760e5b917975fb4058b2..f8af1fccfbf43204a1aaa70500156881d134e65b 100644 (file)
--- a/app/views/user/view.html.erb
+++ b/app/views/user/view.html.erb
@@ -6,23 +6,25 @@
   <% if @user and @this_user.id == @user.id %>
     <!-- Displaying user's own profile page -->
     <%= link_to t('user.view.my edits'), :controller => 'changeset', :action => 'list', :display_name => @user.display_name %>
-    <span class='count-number'><%= number_with_delimiter(@this_user.changesets.size) %></span>
+    <span class='count-number'><%= number_with_delimiter(@user.changesets.size) %></span>
     |
     <%= link_to t('user.view.my traces'), :controller => 'trace', :action=>'mine' %>
-    <span class='count-number'><%= number_with_delimiter(@this_user.traces.size) %></span>
+    <span class='count-number'><%= number_with_delimiter(@user.traces.size) %></span>
     |
     <%= link_to t('user.view.my diary'), :controller => 'diary_entry', :action => 'list', :display_name => @user.display_name %>
     |
-    <%= link_to t('user.view.my comments' ), :controller => 'diary_entry', :action => 'comments', :display_name => @this_user.display_name %>
+    <%= link_to t('user.view.my comments' ), :controller => 'diary_entry', :action => 'comments', :display_name => @user.display_name %>
     |
     <%= link_to t('user.view.my settings'), :controller => 'user', :action => 'account', :display_name => @user.display_name %>
-    <% if @user.active_blocks.count > 0 %>
+    <% if @user.blocks.exists? %>
       |
       <%= link_to t('user.view.blocks on me'), :controller => 'user_blocks', :action => 'blocks_on', :display_name => @user.display_name %>
-      <span class='count-number'><%= number_with_delimiter(@this_user.active_blocks.count) %></span>
+      <span class='count-number'><%= number_with_delimiter(@user.blocks.active.size) %></span>
     <% end %>
-    <% if @user and @user.moderator? %>
-      | <%= link_to t('user.view.blocks by me'), :controller => 'user_blocks', :action => 'blocks_by', :display_name => @user.display_name %>
+    <% if @user and @user.moderator? and @user.blocks_created.exists? %>
+      |
+      <%= link_to t('user.view.blocks by me'), :controller => 'user_blocks', :action => 'blocks_by', :display_name => @user.display_name %>
+      <span class='count-number'><%= number_with_delimiter(@user.blocks_created.active.size) %></span>
     <% end %>
   <% else %>
     <%= link_to t('user.view.edits'), :controller => 'changeset', :action => 'list', :display_name => @this_user.display_name %>
@@ -41,10 +43,15 @@
     <% else %>
       <%= link_to t('user.view.add as friend'), :controller => 'user', :action => 'make_friend', :display_name => @this_user.display_name %>
     <% end %>
-    |
-    <%= link_to t('user.view.block_history'), :controller => 'user_blocks', :action => 'blocks_on', :display_name => @this_user.display_name %>
-    <% if @this_user.moderator? %>
-      | <%= link_to t('user.view.moderator_history'), :controller => 'user_blocks', :action => 'blocks_by', :display_name => @this_user.display_name %>
+    <% if @this_user.blocks.exists? %>
+      |
+      <%= link_to t('user.view.block_history'), :controller => 'user_blocks', :action => 'blocks_on', :display_name => @this_user.display_name %>
+      <span class='count-number'><%= number_with_delimiter(@this_user.blocks.active.size) %></span>
+    <% end %>
+    <% if @this_user.moderator? and @this_user.blocks_created.exists? %>
+      |
+      <%= link_to t('user.view.moderator_history'), :controller => 'user_blocks', :action => 'blocks_by', :display_name => @this_user.display_name %>
+      <span class='count-number'><%= number_with_delimiter(@this_user.blocks_created.active.size) %></span>
     <% end %>
     <% if @user and @user.moderator? %>
       | <%= link_to t('user.view.create_block'), :controller => 'user_blocks', :action => 'new', :display_name => @this_user.display_name %>

diff --git a/config/locales/en.yml b/config/locales/en.yml
index d01482045662ce466895f750d5211002604a1f48..7ecb5c2e97b23f554a940a7f03ba30d97bbc7c70 100644 (file)
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -1742,8 +1742,8 @@ en:
         revoke:
           administrator: "Revoke administrator access"
           moderator: "Revoke moderator access"
-      block_history: "view blocks received"
-      moderator_history: "view blocks given"
+      block_history: "blocks received"
+      moderator_history: "blocks given"
       comments: "comments"
       create_block: "block this user"
       activate_user: "activate this user"