From: Tom Hughes Date: Thu, 9 Aug 2012 09:48:34 +0000 (+0100) Subject: Improve blocks on/by links on user pages X-Git-Tag: live~5445 X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/fbabed694b5acae64c8baf633ab17d1e61010370?hp=ec92881dd019dd6f2581a4d885a74734e0461563 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 a24ac5265..ef7a63639 100644 --- 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 b512a404d..8f9ae2948 100644 --- 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 1b9dd54e7..68537e749 100644 --- 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 8821926bb..2cf0eefc4 100644 --- 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 95b66e864..f8af1fccf 100644 --- 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 %> <%= link_to t('user.view.my edits'), :controller => 'changeset', :action => 'list', :display_name => @user.display_name %> - <%= number_with_delimiter(@this_user.changesets.size) %> + <%= number_with_delimiter(@user.changesets.size) %> | <%= link_to t('user.view.my traces'), :controller => 'trace', :action=>'mine' %> - <%= number_with_delimiter(@this_user.traces.size) %> + <%= number_with_delimiter(@user.traces.size) %> | <%= 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 %> - <%= number_with_delimiter(@this_user.active_blocks.count) %> + <%= number_with_delimiter(@user.blocks.active.size) %> <% 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 %> + <%= number_with_delimiter(@user.blocks_created.active.size) %> <% 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 %> + <%= number_with_delimiter(@this_user.blocks.active.size) %> + <% 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 %> + <%= number_with_delimiter(@this_user.blocks_created.active.size) %> <% 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 d01482045..7ecb5c2e9 100644 --- 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"