From 3227f401935e809edc91d360339220a89567db7a Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Fri, 23 Sep 2011 00:35:25 +0100 Subject: [PATCH] Make more use of named scopes --- app/controllers/diary_entry_controller.rb | 19 +++++++------------ app/controllers/trace_controller.rb | 16 ++++++++-------- app/controllers/user_controller.rb | 8 ++++---- app/models/diary_entry.rb | 2 ++ app/models/trace.rb | 1 + app/models/user.rb | 3 +++ 6 files changed, 25 insertions(+), 24 deletions(-) diff --git a/app/controllers/diary_entry_controller.rb b/app/controllers/diary_entry_controller.rb index b9eb1c2d6..0444f5a89 100644 --- a/app/controllers/diary_entry_controller.rb +++ b/app/controllers/diary_entry_controller.rb @@ -71,7 +71,7 @@ class DiaryEntryController < ApplicationController def list if params[:display_name] - @this_user = User.where(:status => ["active", "confirmed"]).find_by_display_name(params[:display_name]) + @this_user = User.active.find_by_display_name(params[:display_name]) if @this_user @title = t 'diary_entry.list.user_title', :user => @this_user.display_name @@ -114,10 +114,10 @@ class DiaryEntryController < ApplicationController @entries = DiaryEntry.includes(:user).order("created_at DESC").limit(20) if params[:display_name] - user = User.where("status IN ('active', 'confirmed')").find_by_display_name(params[:display_name]) + user = User.active.find_by_display_name(params[:display_name]) if user - @entries = @entries.where(:user_id => user.id, :visible => true ) + @entries = user.diary_entries.visible @title = I18n.t('diary_entry.feed.user.title', :user => user.display_name) @description = I18n.t('diary_entry.feed.user.description', :user => user.display_name) @link = "http://#{SERVER_URL}/user/#{user.display_name}/diary" @@ -125,15 +125,12 @@ class DiaryEntryController < ApplicationController render :nothing => true, :status => :not_found end elsif params[:language] - @entries = @entries.where(:users => { :status => ["active", "confirmed"] }, - :visible => true, - :language_code => params[:language]) + @entries = @entries.visible.where(:language_code => params[:language]).joins(:user).where(:users => { :status => ["active", "confirmed"] }) @title = I18n.t('diary_entry.feed.language.title', :language_name => Language.find(params[:language]).english_name) @description = I18n.t('diary_entry.feed.language.description', :language_name => Language.find(params[:language]).english_name) @link = "http://#{SERVER_URL}/diary/#{params[:language]}" else - @entries = @entries.where(:users => { :status => ["active", "confirmed"] }, - :visible => true) + @entries = @entries.visible.joins(:user).where(:users => { :status => ["active", "confirmed"] }) @title = I18n.t('diary_entry.feed.all.title') @description = I18n.t('diary_entry.feed.all.description') @link = "http://#{SERVER_URL}/diary" @@ -141,12 +138,10 @@ class DiaryEntryController < ApplicationController end def view - user = User.where("status IN ('active', 'confirmed')").find_by_display_name(params[:display_name]) + user = User.active.find_by_display_name(params[:display_name]) if user - @entry = DiaryEntry.where(:id => params[:id], - :user_id => user.id, - :visible => true).first + @entry = user.diary_entries.visible.where(:id => params[:id]).first if @entry @title = t 'diary_entry.view.title', :user => params[:display_name], :title => @entry.title else diff --git a/app/controllers/trace_controller.rb b/app/controllers/trace_controller.rb index 944335a8f..1ca28246f 100644 --- a/app/controllers/trace_controller.rb +++ b/app/controllers/trace_controller.rb @@ -27,7 +27,7 @@ class TraceController < ApplicationController # from display name, pick up user id if one user's traces only display_name = params[:display_name] if !display_name.blank? - target_user = User.where("status IN ('active', 'confirmed')").where(:display_name => display_name).first + target_user = User.active.where(:display_name => display_name).first if target_user.nil? @title = t'trace.no_such_user.title' @not_found_user = display_name @@ -54,15 +54,15 @@ class TraceController < ApplicationController # 4 - user's traces, not logged in as that user = all user's public traces if target_user.nil? # all traces if @user - @traces = Trace.where("visibility IN ('public', 'identifiable') OR user_id = ?", @user.id) #1 + @traces = Trace.visible_to(@user) #1 else - @traces = Trace.where("visibility IN ('public', 'identifiable')") #2 + @traces = Trace.public #2 end else if @user and @user == target_user - @traces = Trace.where(:user_id => @user.id) #3 (check vs user id, so no join + can't pick up non-public traces by changing name) + @traces = @user.traces #3 (check vs user id, so no join + can't pick up non-public traces by changing name) else - @traces = Trace.where("visibility IN ('public', 'identifiable') AND user_id = ?", target_user.id) #4 + @traces = target_user.traces.public #4 end end @@ -79,7 +79,7 @@ class TraceController < ApplicationController @page = (params[:page] || 1).to_i @page_size = 20 - @traces = @traces.where(:visible => true) + @traces = @traces.visible @traces = @traces.order("timestamp DESC") @traces = @traces.offset((@page - 1) * @page_size) @traces = @traces.limit(@page_size) @@ -214,10 +214,10 @@ class TraceController < ApplicationController end def georss - traces = Trace.where("visibility IN ('public', 'identifiable')") + traces = Trace.public if params[:display_name] - traces = traces.where(:users => {:display_name => params[:display_name]}) + traces = traces.joins(:user).where(:users => {:display_name => params[:display_name]}) end if params[:tag] diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index 9bc18c16b..9ef1ab409 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -139,7 +139,7 @@ class UserController < ApplicationController def account @title = t 'user.account.title' - @tokens = @user.oauth_tokens.where('oauth_tokens.invalidated_at is null and oauth_tokens.authorized_at is not null') + @tokens = @user.oauth_tokens.authorized if params[:user] and params[:user][:display_name] and params[:user][:description] @user.display_name = params[:user][:display_name] @@ -208,7 +208,7 @@ class UserController < ApplicationController @title = t 'user.lost_password.title' if params[:user] and params[:user][:email] - user = User.where(:email => params[:user][:email], :status => ["pending", "active", "confirmed"]).first + user = User.visible.where(:email => params[:user][:email]).first if user token = user.tokens.create @@ -410,7 +410,7 @@ class UserController < ApplicationController def make_friend if params[:display_name] name = params[:display_name] - new_friend = User.where(:display_name => name, :status => ["active", "confirmed"]).first + new_friend = User.active.where(:display_name => name).first friend = Friend.new friend.user_id = @user.id friend.friend_user_id = new_friend.id @@ -436,7 +436,7 @@ class UserController < ApplicationController def remove_friend if params[:display_name] name = params[:display_name] - friend = User.where(:display_name => name, :status => ["active", "confirmed"]).first + friend = User.active.where(:display_name => name).first if @user.is_friends_with?(friend) Friend.delete_all "user_id = #{@user.id} AND friend_user_id = #{friend.id}" flash[:notice] = t 'user.remove_friend.success', :name => friend.display_name diff --git a/app/models/diary_entry.rb b/app/models/diary_entry.rb index 9146eb800..318672c03 100644 --- a/app/models/diary_entry.rb +++ b/app/models/diary_entry.rb @@ -13,6 +13,8 @@ class DiaryEntry < ActiveRecord::Base }, :order => "diary_comments.id" + scope :visible, where(:visible => true) + validates_presence_of :title, :body validates_length_of :title, :within => 1..255 #validates_length_of :language, :within => 2..5, :allow_nil => false diff --git a/app/models/trace.rb b/app/models/trace.rb index 45abdf474..3f58cee2e 100644 --- a/app/models/trace.rb +++ b/app/models/trace.rb @@ -7,6 +7,7 @@ class Trace < ActiveRecord::Base scope :visible, where(:visible => true) scope :visible_to, lambda { |u| visible.where("visibility IN ('public', 'identifiable') OR user_id = ?", u) } + scope :public, where(:visibility => ["public", "identifiable"]) validates_presence_of :user_id, :name, :timestamp validates_presence_of :description, :on => :create diff --git a/app/models/user.rb b/app/models/user.rb index 759c8d0b0..d3a05d4bb 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -18,6 +18,9 @@ class User < ActiveRecord::Base 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 :roles, :class_name => "UserRole" + scope :visible, where(:status => ["pending", "active", "confirmed"]) + scope :active, where(:status => ["active", "confirmed"]) + validates_presence_of :email, :display_name validates_confirmation_of :email#, :message => ' addresses must match' validates_confirmation_of :pass_crypt#, :message => ' must match the confirmation password' -- 2.43.2