From f9ddb562d58ae98bcc69f74396b028cbc8cce0b1 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Wed, 12 Jun 2013 19:13:25 +0000 Subject: [PATCH] Cleanup of finders with :conditions option. git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@11963 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/context_menus_controller.rb | 3 +- app/controllers/issues_controller.rb | 2 +- app/controllers/messages_controller.rb | 2 +- app/controllers/projects_controller.rb | 2 +- app/controllers/repositories_controller.rb | 19 +++--- app/controllers/sys_controller.rb | 6 +- app/controllers/users_controller.rb | 2 +- app/controllers/versions_controller.rb | 10 +-- app/helpers/queries_helper.rb | 2 +- app/helpers/versions_helper.rb | 7 +- app/models/issue.rb | 15 ++--- app/models/issue_query.rb | 67 +++++++++++-------- app/models/repository.rb | 25 ++++--- app/models/repository/bazaar.rb | 14 ++-- app/models/repository/cvs.rb | 21 +++--- app/models/repository/git.rb | 17 +---- app/models/wiki.rb | 4 +- app/views/users/show.html.erb | 2 +- lib/redmine/helpers/gantt.rb | 11 +-- .../custom_fields_controller_test.rb | 2 +- test/functional/issues_controller_test.rb | 4 +- test/functional/projects_controller_test.rb | 2 +- .../repositories_controller_test.rb | 2 +- test/functional/workflows_controller_test.rb | 8 +-- .../api_test/issue_categories_test.rb | 4 +- test/integration/api_test/issues_test.rb | 14 ++-- test/test_helper.rb | 2 +- test/unit/board_test.rb | 2 +- test/unit/changeset_test.rb | 6 +- test/unit/issue_status_test.rb | 8 +-- test/unit/project_copy_test.rb | 10 +-- test/unit/project_test.rb | 12 ++-- test/unit/query_test.rb | 2 +- test/unit/repository_test.rb | 6 +- test/unit/search_test.rb | 2 +- test/unit/user_test.rb | 2 +- test/unit/workflow_test.rb | 6 +- 37 files changed, 155 insertions(+), 170 deletions(-) diff --git a/app/controllers/context_menus_controller.rb b/app/controllers/context_menus_controller.rb index d78ef3d3e..8f6fc1cb4 100644 --- a/app/controllers/context_menus_controller.rb +++ b/app/controllers/context_menus_controller.rb @@ -71,8 +71,7 @@ class ContextMenusController < ApplicationController end def time_entries - @time_entries = TimeEntry.all( - :conditions => {:id => params[:ids]}, :include => :project) + @time_entries = TimeEntry.where(:id => params[:ids]).preload(:project).to_a (render_404; return) unless @time_entries.present? @projects = @time_entries.collect(&:project).compact.uniq diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index 327530009..0546db36b 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -301,7 +301,7 @@ class IssuesController < ApplicationController end def destroy - @hours = TimeEntry.sum(:hours, :conditions => ['issue_id IN (?)', @issues]).to_f + @hours = TimeEntry.where(:issue_id => @issues.map(&:id)).sum(:hours).to_f if @hours > 0 case params[:todo] when 'destroy' diff --git a/app/controllers/messages_controller.rb b/app/controllers/messages_controller.rb index ad9107639..585101fbd 100644 --- a/app/controllers/messages_controller.rb +++ b/app/controllers/messages_controller.rb @@ -35,7 +35,7 @@ class MessagesController < ApplicationController page = params[:page] # Find the page of the requested reply if params[:r] && page.nil? - offset = @topic.children.count(:conditions => ["#{Message.table_name}.id < ?", params[:r].to_i]) + offset = @topic.children.where("#{Message.table_name}.id < ?", params[:r].to_i).count page = 1 + offset / REPLIES_PER_PAGE end diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 4b1befdd5..968898411 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -155,7 +155,7 @@ class ProjectsController < ApplicationController @total_issues_by_tracker = Issue.visible.where(cond).count(:group => :tracker) if User.current.allowed_to?(:view_time_entries, @project) - @total_hours = TimeEntry.visible.sum(:hours, :include => :project, :conditions => cond).to_f + @total_hours = TimeEntry.visible.where(cond).sum(:hours).to_f end @key = User.current.rss_key diff --git a/app/controllers/repositories_controller.rb b/app/controllers/repositories_controller.rb index 0e4f4821f..2db9c3f92 100644 --- a/app/controllers/repositories_controller.rb +++ b/app/controllers/repositories_controller.rb @@ -352,15 +352,18 @@ class RepositoriesController < ApplicationController @date_to = Date.today @date_from = @date_to << 11 @date_from = Date.civil(@date_from.year, @date_from.month, 1) - commits_by_day = Changeset.count( - :all, :group => :commit_date, - :conditions => ["repository_id = ? AND commit_date BETWEEN ? AND ?", repository.id, @date_from, @date_to]) + commits_by_day = Changeset. + where("repository_id = ? AND commit_date BETWEEN ? AND ?", repository.id, @date_from, @date_to). + group(:commit_date). + count commits_by_month = [0] * 12 commits_by_day.each {|c| commits_by_month[(@date_to.month - c.first.to_date.month) % 12] += c.last } - changes_by_day = Change.count( - :all, :group => :commit_date, :include => :changeset, - :conditions => ["#{Changeset.table_name}.repository_id = ? AND #{Changeset.table_name}.commit_date BETWEEN ? AND ?", repository.id, @date_from, @date_to]) + changes_by_day = Change. + joins(:changeset). + where("#{Changeset.table_name}.repository_id = ? AND #{Changeset.table_name}.commit_date BETWEEN ? AND ?", repository.id, @date_from, @date_to). + group(:commit_date). + count changes_by_month = [0] * 12 changes_by_day.each {|c| changes_by_month[(@date_to.month - c.first.to_date.month) % 12] += c.last } @@ -393,10 +396,10 @@ class RepositoriesController < ApplicationController end def graph_commits_per_author(repository) - commits_by_author = Changeset.count(:all, :group => :committer, :conditions => ["repository_id = ?", repository.id]) + commits_by_author = Changeset.where("repository_id = ?", repository.id).group(:committer).count commits_by_author.to_a.sort! {|x, y| x.last <=> y.last} - changes_by_author = Change.count(:all, :group => :committer, :include => :changeset, :conditions => ["#{Changeset.table_name}.repository_id = ?", repository.id]) + changes_by_author = Change.joins(:changeset).where("#{Changeset.table_name}.repository_id = ?", repository.id).group(:committer).count h = changes_by_author.inject({}) {|o, i| o[i.first] = i.last; o} fields = commits_by_author.collect {|r| r.first} diff --git a/app/controllers/sys_controller.rb b/app/controllers/sys_controller.rb index b822c5c1e..295f6c030 100644 --- a/app/controllers/sys_controller.rb +++ b/app/controllers/sys_controller.rb @@ -19,11 +19,7 @@ class SysController < ActionController::Base before_filter :check_enabled def projects - p = Project.active.has_module(:repository).find( - :all, - :include => :repository, - :order => "#{Project.table_name}.identifier" - ) + p = Project.active.has_module(:repository).order("#{Project.table_name}.identifier").preload(:repository).all # extra_info attribute from repository breaks activeresource client render :xml => p.to_xml( :only => [:id, :identifier, :name, :is_public, :status], diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 3ea65c7aa..4d89e04f5 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -60,7 +60,7 @@ class UsersController < ApplicationController def show # show projects based on current user visibility - @memberships = @user.memberships.all(:conditions => Project.visible_condition(User.current)) + @memberships = @user.memberships.where(Project.visible_condition(User.current)).all events = Redmine::Activity::Fetcher.new(User.current, :author => @user).events(nil, nil, :limit => 10) @events_by_day = events.group_by(&:event_date) diff --git a/app/controllers/versions_controller.rb b/app/controllers/versions_controller.rb index f159f7ecc..aa5beb0f9 100644 --- a/app/controllers/versions_controller.rb +++ b/app/controllers/versions_controller.rb @@ -46,11 +46,11 @@ class VersionsController < ApplicationController @issues_by_version = {} if @selected_tracker_ids.any? && @versions.any? - issues = Issue.visible.all( - :include => [:project, :status, :tracker, :priority, :fixed_version], - :conditions => {:tracker_id => @selected_tracker_ids, :project_id => project_ids, :fixed_version_id => @versions.map(&:id)}, - :order => "#{Project.table_name}.lft, #{Tracker.table_name}.position, #{Issue.table_name}.id" - ) + issues = Issue.visible. + includes(:project, :tracker). + preload(:status, :priority, :fixed_version). + where(:tracker_id => @selected_tracker_ids, :project_id => project_ids, :fixed_version_id => @versions.map(&:id)). + order("#{Project.table_name}.lft, #{Tracker.table_name}.position, #{Issue.table_name}.id") @issues_by_version = issues.group_by(&:fixed_version) end @versions.reject! {|version| !project_ids.include?(version.project_id) && @issues_by_version[version].blank?} diff --git a/app/helpers/queries_helper.rb b/app/helpers/queries_helper.rb index 2de6b0c90..d92c300e1 100644 --- a/app/helpers/queries_helper.rb +++ b/app/helpers/queries_helper.rb @@ -185,7 +185,7 @@ module QueriesHelper if !params[:query_id].blank? cond = "project_id IS NULL" cond << " OR project_id = #{@project.id}" if @project - @query = IssueQuery.find(params[:query_id], :conditions => cond) + @query = IssueQuery.where(cond).find(params[:query_id]) raise ::Unauthorized unless @query.visible? @query.project = @project session[:query] = {:id => @query.id, :project_id => @query.project_id} diff --git a/app/helpers/versions_helper.rb b/app/helpers/versions_helper.rb index 61cb382c5..082daabd3 100644 --- a/app/helpers/versions_helper.rb +++ b/app/helpers/versions_helper.rb @@ -35,12 +35,9 @@ module VersionsHelper h = Hash.new {|k,v| k[v] = [0, 0]} begin # Total issue count - Issue.count(:group => criteria, - :conditions => ["#{Issue.table_name}.fixed_version_id = ?", version.id]).each {|c,s| h[c][0] = s} + Issue.where(:fixed_version_id => version.id).group(criteria).count.each {|c,s| h[c][0] = s} # Open issues count - Issue.count(:group => criteria, - :include => :status, - :conditions => ["#{Issue.table_name}.fixed_version_id = ? AND #{IssueStatus.table_name}.is_closed = ?", version.id, false]).each {|c,s| h[c][1] = s} + Issue.open.where(:fixed_version_id => version.id).group(criteria).count.each {|c,s| h[c][1] = s} rescue ActiveRecord::RecordNotFound # When grouping by an association, Rails throws this exception if there's no result (bug) end diff --git a/app/models/issue.rb b/app/models/issue.rb index bdb9582b7..e68dc02f7 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -812,7 +812,7 @@ class Issue < ActiveRecord::Base # Preloads relations for a collection of issues def self.load_relations(issues) if issues.any? - relations = IssueRelation.all(:conditions => ["issue_from_id IN (:ids) OR issue_to_id IN (:ids)", {:ids => issues.map(&:id)}]) + relations = IssueRelation.where("issue_from_id IN (:ids) OR issue_to_id IN (:ids)", :ids => issues.map(&:id)).all issues.each do |issue| issue.instance_variable_set "@relations", relations.select {|r| r.issue_from_id == issue.id || r.issue_to_id == issue.id} end @@ -822,7 +822,7 @@ class Issue < ActiveRecord::Base # Preloads visible spent time for a collection of issues def self.load_visible_spent_hours(issues, user=User.current) if issues.any? - hours_by_issue_id = TimeEntry.visible(user).sum(:hours, :group => :issue_id) + hours_by_issue_id = TimeEntry.visible(user).group(:issue_id).sum(:hours) issues.each do |issue| issue.instance_variable_set "@spent_hours", (hours_by_issue_id[issue.id] || 0) end @@ -850,7 +850,7 @@ class Issue < ActiveRecord::Base # Finds an issue relation given its id. def find_relation(relation_id) - IssueRelation.find(relation_id, :conditions => ["issue_to_id = ? OR issue_from_id = ?", id, id]) + IssueRelation.where("issue_to_id = ? OR issue_from_id = ?", id, id).find(relation_id) end # Returns all the other issues that depend on the issue @@ -1350,12 +1350,11 @@ class Issue < ActiveRecord::Base def self.update_versions(conditions=nil) # Only need to update issues with a fixed_version from # a different project and that is not systemwide shared - Issue.scoped(:conditions => conditions).all( - :conditions => "#{Issue.table_name}.fixed_version_id IS NOT NULL" + + Issue.includes(:project, :fixed_version). + where("#{Issue.table_name}.fixed_version_id IS NOT NULL" + " AND #{Issue.table_name}.project_id <> #{Version.table_name}.project_id" + - " AND #{Version.table_name}.sharing <> 'system'", - :include => [:project, :fixed_version] - ).each do |issue| + " AND #{Version.table_name}.sharing <> 'system'"). + where(conditions).each do |issue| next if issue.project.nil? || issue.fixed_version.nil? unless issue.project.shared_versions.include?(issue.fixed_version) issue.init_journal(User.current) diff --git a/app/models/issue_query.rb b/app/models/issue_query.rb index cf6074f72..f9adf4697 100644 --- a/app/models/issue_query.rb +++ b/app/models/issue_query.rb @@ -226,7 +226,7 @@ class IssueQuery < Query # Returns the issue count def issue_count - Issue.visible.count(:include => [:status, :project], :conditions => statement) + Issue.visible.joins(:status, :project).where(statement).count rescue ::ActiveRecord::StatementInvalid => e raise StatementInvalid.new(e.message) end @@ -237,7 +237,12 @@ class IssueQuery < Query if grouped? begin # Rails3 will raise an (unexpected) RecordNotFound if there's only a nil group value - r = Issue.visible.count(:joins => joins_for_order_statement(group_by_statement), :group => group_by_statement, :include => [:status, :project], :conditions => statement) + r = Issue.visible. + joins(:status, :project). + where(statement). + joins(joins_for_order_statement(group_by_statement)). + group(group_by_statement). + count rescue ActiveRecord::RecordNotFound r = {nil => issue_count} end @@ -256,14 +261,16 @@ class IssueQuery < Query def issues(options={}) order_option = [group_by_sort_order, options[:order]].flatten.reject(&:blank?) - issues = Issue.visible.where(options[:conditions]).all( - :include => ([:status, :project] + (options[:include] || [])).uniq, - :conditions => statement, - :order => order_option, - :joins => joins_for_order_statement(order_option.join(',')), - :limit => options[:limit], - :offset => options[:offset] - ) + issues = Issue.visible. + joins(:status, :project). + where(statement). + includes(([:status, :project] + (options[:include] || [])).uniq). + where(options[:conditions]). + order(order_option). + joins(joins_for_order_statement(order_option.join(','))). + limit(options[:limit]). + offset(options[:offset]). + all if has_column?(:spent_hours) Issue.load_visible_spent_hours(issues) @@ -280,12 +287,16 @@ class IssueQuery < Query def issue_ids(options={}) order_option = [group_by_sort_order, options[:order]].flatten.reject(&:blank?) - Issue.visible.scoped(:conditions => options[:conditions]).scoped(:include => ([:status, :project] + (options[:include] || [])).uniq, - :conditions => statement, - :order => order_option, - :joins => joins_for_order_statement(order_option.join(',')), - :limit => options[:limit], - :offset => options[:offset]).find_ids + Issue.visible. + joins(:status, :project). + where(statement). + includes(([:status, :project] + (options[:include] || [])).uniq). + where(options[:conditions]). + order(order_option). + joins(joins_for_order_statement(order_option.join(','))). + limit(options[:limit]). + offset(options[:offset]). + find_ids rescue ::ActiveRecord::StatementInvalid => e raise StatementInvalid.new(e.message) end @@ -293,13 +304,14 @@ class IssueQuery < Query # Returns the journals # Valid options are :order, :offset, :limit def journals(options={}) - Journal.visible.all( - :include => [:details, :user, {:issue => [:project, :author, :tracker, :status]}], - :conditions => statement, - :order => options[:order], - :limit => options[:limit], - :offset => options[:offset] - ) + Journal.visible. + joins(:issue => [:project, :status]). + where(statement). + order(options[:order]). + limit(options[:limit]). + offset(options[:offset]). + preload(:details, :user, {:issue => [:project, :author, :tracker, :status]}). + all rescue ::ActiveRecord::StatementInvalid => e raise StatementInvalid.new(e.message) end @@ -307,10 +319,11 @@ class IssueQuery < Query # Returns the versions # Valid options are :conditions def versions(options={}) - Version.visible.where(options[:conditions]).all( - :include => :project, - :conditions => project_statement - ) + Version.visible. + where(project_statement). + where(options[:conditions]). + includes(:project). + all rescue ::ActiveRecord::StatementInvalid => e raise StatementInvalid.new(e.message) end diff --git a/app/models/repository.rb b/app/models/repository.rb index b0e946365..de3ca99a6 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -249,19 +249,18 @@ class Repository < ActiveRecord::Base # Default behaviour is to search in cached changesets def latest_changesets(path, rev, limit=10) if path.blank? - changesets.find( - :all, - :include => :user, - :order => "#{Changeset.table_name}.committed_on DESC, #{Changeset.table_name}.id DESC", - :limit => limit) + changesets. + reorder("#{Changeset.table_name}.committed_on DESC, #{Changeset.table_name}.id DESC"). + limit(limit). + preload(:user). + all else - filechanges.find( - :all, - :include => {:changeset => :user}, - :conditions => ["path = ?", path.with_leading_slash], - :order => "#{Changeset.table_name}.committed_on DESC, #{Changeset.table_name}.id DESC", - :limit => limit - ).collect(&:changeset) + filechanges. + where("path = ?", path.with_leading_slash). + reorder("#{Changeset.table_name}.committed_on DESC, #{Changeset.table_name}.id DESC"). + limit(limit). + preload(:changeset => :user). + collect(&:changeset) end end @@ -393,7 +392,7 @@ class Repository < ActiveRecord::Base end def set_as_default? - new_record? && project && !Repository.first(:conditions => {:project_id => project.id}) + new_record? && project && Repository.where(:project_id => project.id).empty? end protected diff --git a/app/models/repository/bazaar.rb b/app/models/repository/bazaar.rb index ea20fe37d..135be83ca 100644 --- a/app/models/repository/bazaar.rb +++ b/app/models/repository/bazaar.rb @@ -68,15 +68,11 @@ class Repository::Bazaar < Repository full_path = File.join(root_url, e.path) e.size = File.stat(full_path).size if File.file?(full_path) end - c = Change.find( - :first, - :include => :changeset, - :conditions => [ - "#{Change.table_name}.revision = ? and #{Changeset.table_name}.repository_id = ?", - e.lastrev.revision, - id - ], - :order => "#{Changeset.table_name}.revision DESC") + c = Change. + includes(:changeset). + where("#{Change.table_name}.revision = ? and #{Changeset.table_name}.repository_id = ?", e.lastrev.revision, id). + order("#{Changeset.table_name}.revision DESC"). + first if c e.lastrev.identifier = c.changeset.revision e.lastrev.name = c.changeset.revision diff --git a/app/models/repository/cvs.rb b/app/models/repository/cvs.rb index 155ac10ed..7bae8c091 100644 --- a/app/models/repository/cvs.rb +++ b/app/models/repository/cvs.rb @@ -143,14 +143,11 @@ class Repository::Cvs < Repository ) cmt = Changeset.normalize_comments(revision.message, repo_log_encoding) author_utf8 = Changeset.to_utf8(revision.author, repo_log_encoding) - cs = changesets.find( - :first, - :conditions => { - :committed_on => tmp_time - time_delta .. tmp_time + time_delta, - :committer => author_utf8, - :comments => cmt - } - ) + cs = changesets.where( + :committed_on => tmp_time - time_delta .. tmp_time + time_delta, + :committer => author_utf8, + :comments => cmt + ).first # create a new changeset.... unless cs # we use a temporaray revision number here (just for inserting) @@ -185,10 +182,10 @@ class Repository::Cvs < Repository end # Renumber new changesets in chronological order - Changeset.all( - :order => 'committed_on ASC, id ASC', - :conditions => ["repository_id = ? AND revision LIKE 'tmp%'", id] - ).each do |changeset| + Changeset. + order('committed_on ASC, id ASC'). + where("repository_id = ? AND revision LIKE 'tmp%'", id). + each do |changeset| changeset.update_attribute :revision, next_revision_number end end # transaction diff --git a/app/models/repository/git.rb b/app/models/repository/git.rb index c1f0020eb..9b3617ba9 100644 --- a/app/models/repository/git.rb +++ b/app/models/repository/git.rb @@ -191,13 +191,8 @@ class Repository::Git < Repository offset = 0 revisions_copy = revisions.clone # revisions will change while offset < revisions_copy.size - recent_changesets_slice = changesets.find( - :all, - :conditions => [ - 'scmid IN (?)', - revisions_copy.slice(offset, limit).map{|x| x.scmid} - ] - ) + scmids = revisions_copy.slice(offset, limit).map{|x| x.scmid} + recent_changesets_slice = changesets.where(:scmid => scmids).all # Subtract revisions that redmine already knows about recent_revisions = recent_changesets_slice.map{|c| c.scmid} revisions.reject!{|r| recent_revisions.include?(r.scmid)} @@ -246,13 +241,7 @@ class Repository::Git < Repository revisions = scm.revisions(path, nil, rev, :limit => limit, :all => false) return [] if revisions.nil? || revisions.empty? - changesets.find( - :all, - :conditions => [ - "scmid IN (?)", - revisions.map!{|c| c.scmid} - ] - ) + changesets.where(:scmid => revisions.map {|c| c.scmid}).all end def clear_extra_info_of_changesets diff --git a/app/models/wiki.rb b/app/models/wiki.rb index d58212d68..dabe19a4e 100644 --- a/app/models/wiki.rb +++ b/app/models/wiki.rb @@ -50,10 +50,10 @@ class Wiki < ActiveRecord::Base @page_found_with_redirect = false title = start_page if title.blank? title = Wiki.titleize(title) - page = pages.first(:conditions => ["LOWER(title) = LOWER(?)", title]) + page = pages.where("LOWER(title) = LOWER(?)", title).first if !page && !(options[:with_redirect] == false) # search for a redirect - redirect = redirects.first(:conditions => ["LOWER(title) = LOWER(?)", title]) + redirect = redirects.where("LOWER(title) = LOWER(?)", title).first if redirect page = find_page(redirect.redirects_to, :with_redirect => false) @page_found_with_redirect = true diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index 8c32235b7..3ac92fe07 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -40,7 +40,7 @@ :from => @events_by_day.keys.first %>

-<%=l(:label_reported_issues)%>: <%= Issue.count(:conditions => ["author_id=?", @user.id]) %> +<%=l(:label_reported_issues)%>: <%= Issue.where(:author_id => @user.id).count %>

diff --git a/lib/redmine/helpers/gantt.rb b/lib/redmine/helpers/gantt.rb index 1625801f9..198de20a0 100644 --- a/lib/redmine/helpers/gantt.rb +++ b/lib/redmine/helpers/gantt.rb @@ -162,11 +162,12 @@ module Redmine ids = issues.collect(&:project).uniq.collect(&:id) if ids.any? # All issues projects and their visible ancestors - @projects = Project.visible.all( - :joins => "LEFT JOIN #{Project.table_name} child ON #{Project.table_name}.lft <= child.lft AND #{Project.table_name}.rgt >= child.rgt", - :conditions => ["child.id IN (?)", ids], - :order => "#{Project.table_name}.lft ASC" - ).uniq + @projects = Project.visible. + joins("LEFT JOIN #{Project.table_name} child ON #{Project.table_name}.lft <= child.lft AND #{Project.table_name}.rgt >= child.rgt"). + where("child.id IN (?)", ids). + order("#{Project.table_name}.lft ASC"). + uniq. + all else @projects = [] end diff --git a/test/functional/custom_fields_controller_test.rb b/test/functional/custom_fields_controller_test.rb index c8193c5d4..c7efcfeb6 100644 --- a/test/functional/custom_fields_controller_test.rb +++ b/test/functional/custom_fields_controller_test.rb @@ -153,7 +153,7 @@ class CustomFieldsControllerTest < ActionController::TestCase end def test_destroy - custom_values_count = CustomValue.count(:conditions => {:custom_field_id => 1}) + custom_values_count = CustomValue.where(:custom_field_id => 1).count assert custom_values_count > 0 assert_difference 'CustomField.count', -1 do diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index e9afa9b0f..707b57438 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -3701,7 +3701,7 @@ class IssuesControllerTest < ActionController::TestCase def test_bulk_copy_should_allow_changing_the_issue_attributes # Fixes random test failure with Mysql - # where Issue.all(:limit => 2, :order => 'id desc', :conditions => {:project_id => 2}) + # where Issue.where(:project_id => 2).limit(2).order('id desc') # doesn't return the expected results Issue.delete_all("project_id=2") @@ -3716,7 +3716,7 @@ class IssuesControllerTest < ActionController::TestCase end end - copied_issues = Issue.all(:limit => 2, :order => 'id desc', :conditions => {:project_id => 2}) + copied_issues = Issue.where(:project_id => 2).limit(2).order('id desc').to_a assert_equal 2, copied_issues.size copied_issues.each do |issue| assert_equal 2, issue.project_id, "Project is incorrect" diff --git a/test/functional/projects_controller_test.rb b/test/functional/projects_controller_test.rb index 29de3df3c..37010cc71 100644 --- a/test/functional/projects_controller_test.rb +++ b/test/functional/projects_controller_test.rb @@ -51,7 +51,7 @@ class ProjectsControllerTest < ActionController::TestCase assert_response :success assert_template 'common/feed' assert_select 'feed>title', :text => 'Redmine: Latest projects' - assert_select 'feed>entry', :count => Project.count(:conditions => Project.visible_condition(User.current)) + assert_select 'feed>entry', :count => Project.visible(User.current).count end test "#index by non-admin user with view_time_entries permission should show overall spent time link" do diff --git a/test/functional/repositories_controller_test.rb b/test/functional/repositories_controller_test.rb index 2231e3134..ec1847fcd 100644 --- a/test/functional/repositories_controller_test.rb +++ b/test/functional/repositories_controller_test.rb @@ -280,7 +280,7 @@ class RepositoriesControllerTest < ActionController::TestCase :revision => 100, :comments => 'Committed by foo.' ) - assert_no_difference "Changeset.count(:conditions => 'user_id = 3')" do + assert_no_difference "Changeset.where(:user_id => 3).count" do post :committers, :id => 10, :committers => { '0' => ['foo', '2'], '1' => ['dlopper', '3']} assert_response 302 assert_equal User.find(2), c.reload.user diff --git a/test/functional/workflows_controller_test.rb b/test/functional/workflows_controller_test.rb index d04c3ec0a..f5bf3910b 100644 --- a/test/functional/workflows_controller_test.rb +++ b/test/functional/workflows_controller_test.rb @@ -30,7 +30,7 @@ class WorkflowsControllerTest < ActionController::TestCase assert_response :success assert_template 'index' - count = WorkflowTransition.count(:all, :conditions => 'role_id = 1 AND tracker_id = 2') + count = WorkflowTransition.where(:role_id => 1, :tracker_id => 2).count assert_tag :tag => 'a', :content => count.to_s, :attributes => { :href => '/workflows/edit?role_id=1&tracker_id=2' } end @@ -125,10 +125,10 @@ class WorkflowsControllerTest < ActionController::TestCase end def test_clear_workflow - assert WorkflowTransition.count(:conditions => {:tracker_id => 1, :role_id => 2}) > 0 + assert WorkflowTransition.where(:role_id => 1, :tracker_id => 2).count > 0 - post :edit, :role_id => 2, :tracker_id => 1 - assert_equal 0, WorkflowTransition.count(:conditions => {:tracker_id => 1, :role_id => 2}) + post :edit, :role_id => 1, :tracker_id => 2 + assert_equal 0, WorkflowTransition.where(:role_id => 1, :tracker_id => 2).count end def test_get_permissions diff --git a/test/integration/api_test/issue_categories_test.rb b/test/integration/api_test/issue_categories_test.rb index dc970a65a..a8a2abda3 100644 --- a/test/integration/api_test/issue_categories_test.rb +++ b/test/integration/api_test/issue_categories_test.rb @@ -95,11 +95,11 @@ class Redmine::ApiTest::IssueCategoriesTest < Redmine::ApiTest::Base end test "DELETE /issue_categories/:id.xml should reassign issues with :reassign_to_id param" do - issue_count = Issue.count(:conditions => {:category_id => 1}) + issue_count = Issue.where(:category_id => 1).count assert issue_count > 0 assert_difference 'IssueCategory.count', -1 do - assert_difference 'Issue.count(:conditions => {:category_id => 2})', 3 do + assert_difference 'Issue.where(:category_id => 2).count', 3 do delete '/issue_categories/1.xml', {:reassign_to_id => 2}, credentials('jsmith') end end diff --git a/test/integration/api_test/issues_test.rb b/test/integration/api_test/issues_test.rb index 176c18105..d8f1dad48 100644 --- a/test/integration/api_test/issues_test.rb +++ b/test/integration/api_test/issues_test.rb @@ -138,9 +138,9 @@ class Redmine::ApiTest::IssuesTest < Redmine::ApiTest::Base get '/issues.xml', {:set_filter => 1, :f => ['cf_1'], :op => {:cf_1 => '='}, :v => {:cf_1 => ['MySQL']}} - expected_ids = Issue.visible.all( - :include => :custom_values, - :conditions => {:custom_values => {:custom_field_id => 1, :value => 'MySQL'}}).map(&:id) + expected_ids = Issue.visible. + joins(:custom_values). + where(:custom_values => {:custom_field_id => 1, :value => 'MySQL'}).map(&:id) assert_select 'issues > issue > id', :count => expected_ids.count do |ids| ids.each { |id| assert expected_ids.delete(id.children.first.content.to_i) } end @@ -151,9 +151,9 @@ class Redmine::ApiTest::IssuesTest < Redmine::ApiTest::Base should "show only issues with the custom field value" do get '/issues.xml', { :cf_1 => 'MySQL' } - expected_ids = Issue.visible.all( - :include => :custom_values, - :conditions => {:custom_values => {:custom_field_id => 1, :value => 'MySQL'}}).map(&:id) + expected_ids = Issue.visible. + joins(:custom_values). + where(:custom_values => {:custom_field_id => 1, :value => 'MySQL'}).map(&:id) assert_select 'issues > issue > id', :count => expected_ids.count do |ids| ids.each { |id| assert expected_ids.delete(id.children.first.content.to_i) } @@ -170,7 +170,7 @@ class Redmine::ApiTest::IssuesTest < Redmine::ApiTest::Base should "show only issues with the status_id" do get '/issues.xml?status_id=5' - expected_ids = Issue.visible.all(:conditions => {:status_id => 5}).map(&:id) + expected_ids = Issue.visible.where(:status_id => 5).map(&:id) assert_select 'issues > issue > id', :count => expected_ids.count do |ids| ids.each { |id| assert expected_ids.delete(id.children.first.content.to_i) } diff --git a/test/test_helper.rb b/test/test_helper.rb index 368a4d41b..39826bc2d 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -112,7 +112,7 @@ class ActiveSupport::TestCase end def change_user_password(login, new_password) - user = User.first(:conditions => {:login => login}) + user = User.where(:login => login).first user.password, user.password_confirmation = new_password, new_password user.save! end diff --git a/test/unit/board_test.rb b/test/unit/board_test.rb index ff4779081..bebb14b02 100644 --- a/test/unit/board_test.rb +++ b/test/unit/board_test.rb @@ -100,7 +100,7 @@ class BoardTest < ActiveSupport::TestCase end end end - assert_equal 0, Message.count(:conditions => {:board_id => 1}) + assert_equal 0, Message.where(:board_id => 1).count end def test_destroy_should_nullify_children diff --git a/test/unit/changeset_test.rb b/test/unit/changeset_test.rb index a652c9810..a97dc37d2 100644 --- a/test/unit/changeset_test.rb +++ b/test/unit/changeset_test.rb @@ -30,8 +30,7 @@ class ChangesetTest < ActiveSupport::TestCase def test_ref_keywords_any ActionMailer::Base.deliveries.clear - Setting.commit_fix_status_id = IssueStatus.find( - :first, :conditions => ["is_closed = ?", true]).id + Setting.commit_fix_status_id = IssueStatus.where(:is_closed => true).first.id Setting.commit_fix_done_ratio = '90' Setting.commit_ref_keywords = '*' Setting.commit_fix_keywords = 'fixes , closes' @@ -113,8 +112,7 @@ class ChangesetTest < ActiveSupport::TestCase end def test_ref_keywords_closing_with_timelog - Setting.commit_fix_status_id = IssueStatus.find( - :first, :conditions => ["is_closed = ?", true]).id + Setting.commit_fix_status_id = IssueStatus.where(:is_closed => true).first.id Setting.commit_ref_keywords = '*' Setting.commit_fix_keywords = 'fixes , closes' Setting.commit_logtime_enabled = '1' diff --git a/test/unit/issue_status_test.rb b/test/unit/issue_status_test.rb index 9c4593244..4ab94c14a 100644 --- a/test/unit/issue_status_test.rb +++ b/test/unit/issue_status_test.rb @@ -36,8 +36,8 @@ class IssueStatusTest < ActiveSupport::TestCase assert_difference 'IssueStatus.count', -1 do assert status.destroy end - assert_nil WorkflowTransition.first(:conditions => {:old_status_id => status.id}) - assert_nil WorkflowTransition.first(:conditions => {:new_status_id => status.id}) + assert_nil WorkflowTransition.where(:old_status_id => status.id).first + assert_nil WorkflowTransition.where(:new_status_id => status.id).first end def test_destroy_status_in_use @@ -98,7 +98,7 @@ class IssueStatusTest < ActiveSupport::TestCase with_settings :issue_done_ratio => 'issue_field' do IssueStatus.update_issue_done_ratios - assert_equal 0, Issue.count(:conditions => {:done_ratio => 50}) + assert_equal 0, Issue.where(:done_ratio => 50).count end end @@ -107,7 +107,7 @@ class IssueStatusTest < ActiveSupport::TestCase with_settings :issue_done_ratio => 'issue_status' do IssueStatus.update_issue_done_ratios - issues = Issue.all(:conditions => {:status_id => 1}) + issues = Issue.where(:status_id => 1).all assert_equal [50], issues.map {|issue| issue.read_attribute(:done_ratio)}.uniq end end diff --git a/test/unit/project_copy_test.rb b/test/unit/project_copy_test.rb index 893e6653f..7499c07dc 100644 --- a/test/unit/project_copy_test.rb +++ b/test/unit/project_copy_test.rb @@ -63,7 +63,7 @@ class ProjectCopyTest < ActiveSupport::TestCase assert_equal @project, issue.project end - copied_issue = @project.issues.first(:conditions => {:subject => "copy issue status"}) + copied_issue = @project.issues.where(:subject => "copy issue status").first assert copied_issue assert copied_issue.status assert_equal "Closed", copied_issue.status.name @@ -93,7 +93,7 @@ class ProjectCopyTest < ActiveSupport::TestCase assert @project.copy(@source_project) @project.reload - copied_issue = @project.issues.first(:conditions => {:subject => "copy issues assigned to a locked version"}) + copied_issue = @project.issues.where(:subject => "copy issues assigned to a locked version").first assert copied_issue assert copied_issue.fixed_version @@ -112,7 +112,7 @@ class ProjectCopyTest < ActiveSupport::TestCase assert @project.copy(@source_project) @project.reload - copied_issue = @project.issues.first(:conditions => {:subject => "change the new issues to use the copied version"}) + copied_issue = @project.issues.where(:subject => "change the new issues to use the copied version").first assert copied_issue assert copied_issue.fixed_version @@ -128,7 +128,7 @@ class ProjectCopyTest < ActiveSupport::TestCase assert @project.copy(@source_project) @project.reload - copied_issue = @project.issues.first(:conditions => {:subject => "keep target shared versions"}) + copied_issue = @project.issues.where(:subject => "keep target shared versions").first assert copied_issue assert_equal assigned_version, copied_issue.fixed_version @@ -175,7 +175,7 @@ class ProjectCopyTest < ActiveSupport::TestCase @source_project.issues << issue assert @project.copy(@source_project) - copied_issue = @project.issues.first(:conditions => {:subject => "copy with attachment"}) + copied_issue = @project.issues.where(:subject => "copy with attachment").first assert_not_nil copied_issue assert_equal 1, copied_issue.attachments.count, "Attachment not copied" assert_equal "testfile.txt", copied_issue.attachments.first.filename diff --git a/test/unit/project_test.rb b/test/unit/project_test.rb index c6ba4cd72..88e99c439 100644 --- a/test/unit/project_test.rb +++ b/test/unit/project_test.rb @@ -175,7 +175,7 @@ class ProjectTest < ActiveSupport::TestCase # Assign an issue of a project to a version of a child project Issue.find(4).update_attribute :fixed_version_id, 4 - assert_no_difference "Project.count(:all, :conditions => 'status = #{Project::STATUS_ARCHIVED}')" do + assert_no_difference "Project.where(:status => Project::STATUS_ARCHIVED).count" do assert_equal false, @ecookbook.archive end @ecookbook.reload @@ -211,9 +211,9 @@ class ProjectTest < ActiveSupport::TestCase # make sure that the project non longer exists assert_raise(ActiveRecord::RecordNotFound) { Project.find(@ecookbook.id) } # make sure related data was removed - assert_nil Member.first(:conditions => {:project_id => @ecookbook.id}) - assert_nil Board.first(:conditions => {:project_id => @ecookbook.id}) - assert_nil Issue.first(:conditions => {:project_id => @ecookbook.id}) + assert_nil Member.where(:project_id => @ecookbook.id).first + assert_nil Board.where(:project_id => @ecookbook.id).first + assert_nil Issue.where(:project_id => @ecookbook.id).first end def test_destroy_should_destroy_subtasks @@ -246,7 +246,7 @@ class ProjectTest < ActiveSupport::TestCase assert_equal 0, Board.count assert_equal 0, Message.count assert_equal 0, News.count - assert_equal 0, Query.count(:conditions => "project_id IS NOT NULL") + assert_equal 0, Query.where("project_id IS NOT NULL").count assert_equal 0, Repository.count assert_equal 0, Changeset.count assert_equal 0, Change.count @@ -260,7 +260,7 @@ class ProjectTest < ActiveSupport::TestCase assert_equal 0, WikiContent::Version.count assert_equal 0, Project.connection.select_all("SELECT * FROM projects_trackers").size assert_equal 0, Project.connection.select_all("SELECT * FROM custom_fields_projects").size - assert_equal 0, CustomValue.count(:conditions => {:customized_type => ['Project', 'Issue', 'TimeEntry', 'Version']}) + assert_equal 0, CustomValue.where(:customized_type => ['Project', 'Issue', 'TimeEntry', 'Version']).count end def test_move_an_orphan_project_to_a_root_project diff --git a/test/unit/query_test.rb b/test/unit/query_test.rb index f3402ce17..59639384f 100644 --- a/test/unit/query_test.rb +++ b/test/unit/query_test.rb @@ -586,7 +586,7 @@ class QueryTest < ActiveSupport::TestCase query = IssueQuery.new(:name => '_', :filters => { 'assigned_to_id' => {:operator => '=', :values => ['me']}}) result = query.issues - assert_equal Issue.visible.all(:conditions => {:assigned_to_id => ([2] + user.reload.group_ids)}).sort_by(&:id), result.sort_by(&:id) + assert_equal Issue.visible.where(:assigned_to_id => ([2] + user.reload.group_ids)).sort_by(&:id), result.sort_by(&:id) assert result.include?(i1) assert result.include?(i2) diff --git a/test/unit/repository_test.rb b/test/unit/repository_test.rb index 2bd430a4d..390a5334e 100644 --- a/test/unit/repository_test.rb +++ b/test/unit/repository_test.rb @@ -183,9 +183,7 @@ class RepositoryTest < ActiveSupport::TestCase Setting.default_language = 'en' # choosing a status to apply to fix issues - Setting.commit_fix_status_id = IssueStatus.find( - :first, - :conditions => ["is_closed = ?", true]).id + Setting.commit_fix_status_id = IssueStatus.where(:is_closed => true).first.id Setting.commit_fix_done_ratio = "90" Setting.commit_ref_keywords = 'refs , references, IssueID' Setting.commit_fix_keywords = 'fixes , closes' @@ -278,7 +276,7 @@ class RepositoryTest < ActiveSupport::TestCase end def test_manual_user_mapping - assert_no_difference "Changeset.count(:conditions => 'user_id <> 2')" do + assert_no_difference "Changeset.where('user_id <> 2').count" do c = Changeset.create!( :repository => @repository, :committer => 'foo', diff --git a/test/unit/search_test.rb b/test/unit/search_test.rb index 7b124c1f9..fda9ea56f 100644 --- a/test/unit/search_test.rb +++ b/test/unit/search_test.rb @@ -128,7 +128,7 @@ class SearchTest < ActiveSupport::TestCase def test_search_issue_with_multiple_hits_in_journals i = Issue.find(1) - assert_equal 2, i.journals.count(:all, :conditions => "notes LIKE '%notes%'") + assert_equal 2, i.journals.where("notes LIKE '%notes%'").count r = Issue.search('%notes%').first assert_equal 1, r.size diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb index c5b642970..4594d8e1a 100644 --- a/test/unit/user_test.rb +++ b/test/unit/user_test.rb @@ -40,7 +40,7 @@ class UserTest < ActiveSupport::TestCase def test_generate User.generate!(:firstname => 'Testing connection') User.generate!(:firstname => 'Testing connection') - assert_equal 2, User.count(:all, :conditions => {:firstname => 'Testing connection'}) + assert_equal 2, User.where(:firstname => 'Testing connection').count end def test_truth diff --git a/test/unit/workflow_test.rb b/test/unit/workflow_test.rb index e1a517e72..0e4b5ce96 100644 --- a/test/unit/workflow_test.rb +++ b/test/unit/workflow_test.rb @@ -30,9 +30,9 @@ class WorkflowTest < ActiveSupport::TestCase WorkflowTransition.copy(Tracker.find(2), Role.find(1), Tracker.find(3), Role.find(2)) end - assert WorkflowTransition.first(:conditions => {:role_id => 2, :tracker_id => 3, :old_status_id => 1, :new_status_id => 2, :author => false, :assignee => false}) - assert WorkflowTransition.first(:conditions => {:role_id => 2, :tracker_id => 3, :old_status_id => 1, :new_status_id => 3, :author => false, :assignee => true}) - assert WorkflowTransition.first(:conditions => {:role_id => 2, :tracker_id => 3, :old_status_id => 1, :new_status_id => 4, :author => true, :assignee => false}) + assert WorkflowTransition.where(:role_id => 2, :tracker_id => 3, :old_status_id => 1, :new_status_id => 2, :author => false, :assignee => false).first + assert WorkflowTransition.where(:role_id => 2, :tracker_id => 3, :old_status_id => 1, :new_status_id => 3, :author => false, :assignee => true).first + assert WorkflowTransition.where(:role_id => 2, :tracker_id => 3, :old_status_id => 1, :new_status_id => 4, :author => true, :assignee => false).first end def test_workflow_permission_should_validate_rule