Refactor: extract TimelogController#report to a new controller class

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@4232 e93f8b46-1217-0410-a6f0-8f06a7374b81

app/controllers/time_entry_reports_controller.rb

@@ -0,0 +1,209 @@
+class TimeEntryReportsController < ApplicationController
+  menu_item :issues
+  before_filter :find_optional_project
+  before_filter :load_available_criterias
+
+  helper :sort
+  include SortHelper
+  helper :issues
+  helper :timelog
+  include TimelogHelper
+  helper :custom_fields
+  include CustomFieldsHelper
+
+  def report
+    @criterias = params[:criterias] || []
+    @criterias = @criterias.select{|criteria| @available_criterias.has_key? criteria}
+    @criterias.uniq!
+    @criterias = @criterias[0,3]
+    
+    @columns = (params[:columns] && %w(year month week day).include?(params[:columns])) ? params[:columns] : 'month'
+    
+    retrieve_date_range
+    
+    unless @criterias.empty?
+      sql_select = @criterias.collect{|criteria| @available_criterias[criteria][:sql] + " AS " + criteria}.join(', ')
+      sql_group_by = @criterias.collect{|criteria| @available_criterias[criteria][:sql]}.join(', ')
+      sql_condition = ''
+      
+      if @project.nil?
+        sql_condition = Project.allowed_to_condition(User.current, :view_time_entries)
+      elsif @issue.nil?
+        sql_condition = @project.project_condition(Setting.display_subprojects_issues?)
+      else
+        sql_condition = "#{Issue.table_name}.root_id = #{@issue.root_id} AND #{Issue.table_name}.lft >= #{@issue.lft} AND #{Issue.table_name}.rgt <= #{@issue.rgt}"
+      end
+
+      sql = "SELECT #{sql_select}, tyear, tmonth, tweek, spent_on, SUM(hours) AS hours"
+      sql << " FROM #{TimeEntry.table_name}"
+      sql << time_report_joins
+      sql << " WHERE"
+      sql << " (%s) AND" % sql_condition
+      sql << " (spent_on BETWEEN '%s' AND '%s')" % [ActiveRecord::Base.connection.quoted_date(@from), ActiveRecord::Base.connection.quoted_date(@to)]
+      sql << " GROUP BY #{sql_group_by}, tyear, tmonth, tweek, spent_on"
+      
+      @hours = ActiveRecord::Base.connection.select_all(sql)
+      
+      @hours.each do |row|
+        case @columns
+        when 'year'
+          row['year'] = row['tyear']
+        when 'month'
+          row['month'] = "#{row['tyear']}-#{row['tmonth']}"
+        when 'week'
+          row['week'] = "#{row['tyear']}-#{row['tweek']}"
+        when 'day'
+          row['day'] = "#{row['spent_on']}"
+        end
+      end
+      
+      @total_hours = @hours.inject(0) {|s,k| s = s + k['hours'].to_f}
+      
+      @periods = []
+      # Date#at_beginning_of_ not supported in Rails 1.2.x
+      date_from = @from.to_time
+      # 100 columns max
+      while date_from <= @to.to_time && @periods.length < 100
+        case @columns
+        when 'year'
+          @periods << "#{date_from.year}"
+          date_from = (date_from + 1.year).at_beginning_of_year
+        when 'month'
+          @periods << "#{date_from.year}-#{date_from.month}"
+          date_from = (date_from + 1.month).at_beginning_of_month
+        when 'week'
+          @periods << "#{date_from.year}-#{date_from.to_date.cweek}"
+          date_from = (date_from + 7.day).at_beginning_of_week
+        when 'day'
+          @periods << "#{date_from.to_date}"
+          date_from = date_from + 1.day
+        end
+      end
+    end
+    
+    respond_to do |format|
+      format.html { render :layout => !request.xhr? }
+      format.csv  { send_data(report_to_csv(@criterias, @periods, @hours), :type => 'text/csv; header=present', :filename => 'timelog.csv') }
+    end
+  end
+  
+  private
+
+  # TODO: duplicated in TimelogController
+  def find_optional_project
+    if !params[:issue_id].blank?
+      @issue = Issue.find(params[:issue_id])
+      @project = @issue.project
+    elsif !params[:project_id].blank?
+      @project = Project.find(params[:project_id])
+    end
+    deny_access unless User.current.allowed_to?(:view_time_entries, @project, :global => true)
+  end
+
+  # Retrieves the date range based on predefined ranges or specific from/to param dates
+  # TODO: duplicated in TimelogController
+  def retrieve_date_range
+    @free_period = false
+    @from, @to = nil, nil
+
+    if params[:period_type] == '1' || (params[:period_type].nil? && !params[:period].nil?)
+      case params[:period].to_s
+      when 'today'
+        @from = @to = Date.today
+      when 'yesterday'
+        @from = @to = Date.today - 1
+      when 'current_week'
+        @from = Date.today - (Date.today.cwday - 1)%7
+        @to = @from + 6
+      when 'last_week'
+        @from = Date.today - 7 - (Date.today.cwday - 1)%7
+        @to = @from + 6
+      when '7_days'
+        @from = Date.today - 7
+        @to = Date.today
+      when 'current_month'
+        @from = Date.civil(Date.today.year, Date.today.month, 1)
+        @to = (@from >> 1) - 1
+      when 'last_month'
+        @from = Date.civil(Date.today.year, Date.today.month, 1) << 1
+        @to = (@from >> 1) - 1
+      when '30_days'
+        @from = Date.today - 30
+        @to = Date.today
+      when 'current_year'
+        @from = Date.civil(Date.today.year, 1, 1)
+        @to = Date.civil(Date.today.year, 12, 31)
+      end
+    elsif params[:period_type] == '2' || (params[:period_type].nil? && (!params[:from].nil? || !params[:to].nil?))
+      begin; @from = params[:from].to_s.to_date unless params[:from].blank?; rescue; end
+      begin; @to = params[:to].to_s.to_date unless params[:to].blank?; rescue; end
+      @free_period = true
+    else
+      # default
+    end
+    
+    @from, @to = @to, @from if @from && @to && @from > @to
+    @from ||= (TimeEntry.earilest_date_for_project(@project) || Date.today)
+    @to   ||= (TimeEntry.latest_date_for_project(@project) || Date.today)
+  end
+
+  def load_available_criterias
+    @available_criterias = { 'project' => {:sql => "#{TimeEntry.table_name}.project_id",
+                                          :klass => Project,
+                                          :label => :label_project},
+                             'version' => {:sql => "#{Issue.table_name}.fixed_version_id",
+                                          :klass => Version,
+                                          :label => :label_version},
+                             'category' => {:sql => "#{Issue.table_name}.category_id",
+                                            :klass => IssueCategory,
+                                            :label => :field_category},
+                             'member' => {:sql => "#{TimeEntry.table_name}.user_id",
+                                         :klass => User,
+                                         :label => :label_member},
+                             'tracker' => {:sql => "#{Issue.table_name}.tracker_id",
+                                          :klass => Tracker,
+                                          :label => :label_tracker},
+                             'activity' => {:sql => "#{TimeEntry.table_name}.activity_id",
+                                           :klass => TimeEntryActivity,
+                                           :label => :label_activity},
+                             'issue' => {:sql => "#{TimeEntry.table_name}.issue_id",
+                                         :klass => Issue,
+                                         :label => :label_issue}
+                           }
+    
+    # Add list and boolean custom fields as available criterias
+    custom_fields = (@project.nil? ? IssueCustomField.for_all : @project.all_issue_custom_fields)
+    custom_fields.select {|cf| %w(list bool).include? cf.field_format }.each do |cf|
+      @available_criterias["cf_#{cf.id}"] = {:sql => "(SELECT c.value FROM #{CustomValue.table_name} c WHERE c.custom_field_id = #{cf.id} AND c.customized_type = 'Issue' AND c.customized_id = #{Issue.table_name}.id)",
+                                             :format => cf.field_format,
+                                             :label => cf.name}
+    end if @project
+    
+    # Add list and boolean time entry custom fields
+    TimeEntryCustomField.find(:all).select {|cf| %w(list bool).include? cf.field_format }.each do |cf|
+      @available_criterias["cf_#{cf.id}"] = {:sql => "(SELECT c.value FROM #{CustomValue.table_name} c WHERE c.custom_field_id = #{cf.id} AND c.customized_type = 'TimeEntry' AND c.customized_id = #{TimeEntry.table_name}.id)",
+                                             :format => cf.field_format,
+                                             :label => cf.name}
+    end
+
+    # Add list and boolean time entry activity custom fields
+    TimeEntryActivityCustomField.find(:all).select {|cf| %w(list bool).include? cf.field_format }.each do |cf|
+      @available_criterias["cf_#{cf.id}"] = {:sql => "(SELECT c.value FROM #{CustomValue.table_name} c WHERE c.custom_field_id = #{cf.id} AND c.customized_type = 'Enumeration' AND c.customized_id = #{TimeEntry.table_name}.activity_id)",
+                                             :format => cf.field_format,
+                                             :label => cf.name}
+    end
+
+    call_hook(:controller_timelog_available_criterias, { :available_criterias => @available_criterias, :project => @project })
+    @available_criterias
+  end
+
+  def time_report_joins
+    sql = ''
+    sql << " LEFT JOIN #{Issue.table_name} ON #{TimeEntry.table_name}.issue_id = #{Issue.table_name}.id"
+    sql << " LEFT JOIN #{Project.table_name} ON #{TimeEntry.table_name}.project_id = #{Project.table_name}.id"
+    # TODO: rename hook
+    call_hook(:controller_timelog_time_report_joins, {:sql => sql} )
+    sql
+  end
+
+end

app/controllers/timelog_controller.rb

@@ -18,8 +18,7 @@
 class TimelogController < ApplicationController
   menu_item :issues
   before_filter :find_project, :authorize, :only => [:edit, :destroy]
-  before_filter :find_optional_project, :only => [:report, :details]
-  before_filter :load_available_criterias, :only => [:report]
+  before_filter :find_optional_project, :only => [:details]
 
   verify :method => :post, :only => :destroy, :redirect_to => { :action => :details }
   
@@ -30,82 +29,6 @@ class TimelogController < ApplicationController
   helper :custom_fields
   include CustomFieldsHelper
   
-  def report
-    @criterias = params[:criterias] || []
-    @criterias = @criterias.select{|criteria| @available_criterias.has_key? criteria}
-    @criterias.uniq!
-    @criterias = @criterias[0,3]
-    
-    @columns = (params[:columns] && %w(year month week day).include?(params[:columns])) ? params[:columns] : 'month'
-    
-    retrieve_date_range
-    
-    unless @criterias.empty?
-      sql_select = @criterias.collect{|criteria| @available_criterias[criteria][:sql] + " AS " + criteria}.join(', ')
-      sql_group_by = @criterias.collect{|criteria| @available_criterias[criteria][:sql]}.join(', ')
-      sql_condition = ''
-      
-      if @project.nil?
-        sql_condition = Project.allowed_to_condition(User.current, :view_time_entries)
-      elsif @issue.nil?
-        sql_condition = @project.project_condition(Setting.display_subprojects_issues?)
-      else
-        sql_condition = "#{Issue.table_name}.root_id = #{@issue.root_id} AND #{Issue.table_name}.lft >= #{@issue.lft} AND #{Issue.table_name}.rgt <= #{@issue.rgt}"
-      end
-
-      sql = "SELECT #{sql_select}, tyear, tmonth, tweek, spent_on, SUM(hours) AS hours"
-      sql << " FROM #{TimeEntry.table_name}"
-      sql << time_report_joins
-      sql << " WHERE"
-      sql << " (%s) AND" % sql_condition
-      sql << " (spent_on BETWEEN '%s' AND '%s')" % [ActiveRecord::Base.connection.quoted_date(@from), ActiveRecord::Base.connection.quoted_date(@to)]
-      sql << " GROUP BY #{sql_group_by}, tyear, tmonth, tweek, spent_on"
-      
-      @hours = ActiveRecord::Base.connection.select_all(sql)
-      
-      @hours.each do |row|
-        case @columns
-        when 'year'
-          row['year'] = row['tyear']
-        when 'month'
-          row['month'] = "#{row['tyear']}-#{row['tmonth']}"
-        when 'week'
-          row['week'] = "#{row['tyear']}-#{row['tweek']}"
-        when 'day'
-          row['day'] = "#{row['spent_on']}"
-        end
-      end
-      
-      @total_hours = @hours.inject(0) {|s,k| s = s + k['hours'].to_f}
-      
-      @periods = []
-      # Date#at_beginning_of_ not supported in Rails 1.2.x
-      date_from = @from.to_time
-      # 100 columns max
-      while date_from <= @to.to_time && @periods.length < 100
-        case @columns
-        when 'year'
-          @periods << "#{date_from.year}"
-          date_from = (date_from + 1.year).at_beginning_of_year
-        when 'month'
-          @periods << "#{date_from.year}-#{date_from.month}"
-          date_from = (date_from + 1.month).at_beginning_of_month
-        when 'week'
-          @periods << "#{date_from.year}-#{date_from.to_date.cweek}"
-          date_from = (date_from + 7.day).at_beginning_of_week
-        when 'day'
-          @periods << "#{date_from.to_date}"
-          date_from = date_from + 1.day
-        end
-      end
-    end
-    
-    respond_to do |format|
-      format.html { render :layout => !request.xhr? }
-      format.csv  { send_data(report_to_csv(@criterias, @periods, @hours), :type => 'text/csv; header=present', :filename => 'timelog.csv') }
-    end
-  end
-  
   def details
     sort_init 'spent_on', 'desc'
     sort_update 'spent_on' => 'spent_on',
@@ -264,61 +187,4 @@ private
     @to   ||= (TimeEntry.latest_date_for_project(@project) || Date.today)
   end
 
-  def load_available_criterias
-    @available_criterias = { 'project' => {:sql => "#{TimeEntry.table_name}.project_id",
-                                          :klass => Project,
-                                          :label => :label_project},
-                             'version' => {:sql => "#{Issue.table_name}.fixed_version_id",
-                                          :klass => Version,
-                                          :label => :label_version},
-                             'category' => {:sql => "#{Issue.table_name}.category_id",
-                                            :klass => IssueCategory,
-                                            :label => :field_category},
-                             'member' => {:sql => "#{TimeEntry.table_name}.user_id",
-                                         :klass => User,
-                                         :label => :label_member},
-                             'tracker' => {:sql => "#{Issue.table_name}.tracker_id",
-                                          :klass => Tracker,
-                                          :label => :label_tracker},
-                             'activity' => {:sql => "#{TimeEntry.table_name}.activity_id",
-                                           :klass => TimeEntryActivity,
-                                           :label => :label_activity},
-                             'issue' => {:sql => "#{TimeEntry.table_name}.issue_id",
-                                         :klass => Issue,
-                                         :label => :label_issue}
-                           }
-    
-    # Add list and boolean custom fields as available criterias
-    custom_fields = (@project.nil? ? IssueCustomField.for_all : @project.all_issue_custom_fields)
-    custom_fields.select {|cf| %w(list bool).include? cf.field_format }.each do |cf|
-      @available_criterias["cf_#{cf.id}"] = {:sql => "(SELECT c.value FROM #{CustomValue.table_name} c WHERE c.custom_field_id = #{cf.id} AND c.customized_type = 'Issue' AND c.customized_id = #{Issue.table_name}.id)",
-                                             :format => cf.field_format,
-                                             :label => cf.name}
-    end if @project
-    
-    # Add list and boolean time entry custom fields
-    TimeEntryCustomField.find(:all).select {|cf| %w(list bool).include? cf.field_format }.each do |cf|
-      @available_criterias["cf_#{cf.id}"] = {:sql => "(SELECT c.value FROM #{CustomValue.table_name} c WHERE c.custom_field_id = #{cf.id} AND c.customized_type = 'TimeEntry' AND c.customized_id = #{TimeEntry.table_name}.id)",
-                                             :format => cf.field_format,
-                                             :label => cf.name}
-    end
-
-    # Add list and boolean time entry activity custom fields
-    TimeEntryActivityCustomField.find(:all).select {|cf| %w(list bool).include? cf.field_format }.each do |cf|
-      @available_criterias["cf_#{cf.id}"] = {:sql => "(SELECT c.value FROM #{CustomValue.table_name} c WHERE c.custom_field_id = #{cf.id} AND c.customized_type = 'Enumeration' AND c.customized_id = #{TimeEntry.table_name}.activity_id)",
-                                             :format => cf.field_format,
-                                             :label => cf.name}
-    end
-
-    call_hook(:controller_timelog_available_criterias, { :available_criterias => @available_criterias, :project => @project })
-    @available_criterias
-  end
-
-  def time_report_joins
-    sql = ''
-    sql << " LEFT JOIN #{Issue.table_name} ON #{TimeEntry.table_name}.issue_id = #{Issue.table_name}.id"
-    sql << " LEFT JOIN #{Project.table_name} ON #{TimeEntry.table_name}.project_id = #{Project.table_name}.id"
-    call_hook(:controller_timelog_time_report_joins, {:sql => sql} )
-    sql
-  end
 end

app/views/projects/show.rhtml

@@ -68,7 +68,7 @@
     <h3><%= l(:label_spent_time) %></h3>
     <p><span class="icon icon-time"><%= l_hours(@total_hours) %></span></p>
     <p><%= link_to(l(:label_details), {:controller => 'timelog', :action => 'details', :project_id => @project}) %> |
-    <%= link_to(l(:label_report), {:controller => 'timelog', :action => 'report', :project_id => @project}) %></p>
+    <%= link_to(l(:label_report), {:controller => 'time_entry_reports', :action => 'report', :project_id => @project}) %></p>
     <% end %>
     <%= call_hook(:view_projects_show_sidebar_bottom, :project => @project) %>
 <% end %>

app/views/time_entry_reports/report.rhtml

@@ -13,7 +13,7 @@
   <%# TODO: get rid of the project_id field, that should already be in the URL %>
   <%= hidden_field_tag('project_id', params[:project_id]) if @project %>
   <%= hidden_field_tag('issue_id', params[:issue_id]) if @issue %>
-  <%= render :partial => 'date_range' %>
+  <%= render :partial => 'timelog/date_range' %>
 
   <p><%= l(:label_details) %>: <%= select_tag 'columns', options_for_select([[l(:label_year), 'year'],
                                                                             [l(:label_month), 'month'],

app/views/timelog/_date_range.rhtml

@@ -30,7 +30,7 @@
 <ul>
     <li><%= link_to(l(:label_details), url_params.merge({:controller => 'timelog', :action => 'details', :project_id => @project, :issue_id => @issue }),
                                        :class => (@controller.action_name == 'details' ? 'selected' : nil)) %></li>
-    <li><%= link_to(l(:label_report), url_params.merge({:controller => 'timelog', :action => 'report', :project_id => @project, :issue_id => @issue}),
+    <li><%= link_to(l(:label_report), url_params.merge({:controller => 'time_entry_reports', :action => 'report', :project_id => @project, :issue_id => @issue}),
                                        :class => (@controller.action_name == 'report' ? 'selected' : nil)) %></li>
 </ul>
 </div>

config/routes.rb

@@ -30,8 +30,8 @@ ActionController::Routing::Routes.draw do |map|
       time_details.connect 'projects/:project_id/issues/:issue_id/time_entries'
       time_details.connect 'projects/:project_id/issues/:issue_id/time_entries.:format'
     end
-    timelog.connect 'projects/:project_id/time_entries/report', :action => 'report'
-    timelog.with_options :action => 'report',:conditions => {:method => :get} do |time_report|
+    timelog.connect 'projects/:project_id/time_entries/report', :controller => 'time_entry_reports', :action => 'report'
+    timelog.with_options :controller => 'time_entry_reports', :action => 'report',:conditions => {:method => :get} do |time_report|
       time_report.connect 'time_entries/report'
       time_report.connect 'time_entries/report.:format'
       time_report.connect 'projects/:project_id/time_entries/report.:format'

lib/redmine.rb

@@ -85,7 +85,7 @@ Redmine::AccessControl.map do |map|
   
   map.project_module :time_tracking do |map|
     map.permission :log_time, {:timelog => :edit}, :require => :loggedin
-    map.permission :view_time_entries, :timelog => [:details, :report]
+    map.permission :view_time_entries, :timelog => [:details], :time_entry_reports => [:report]
     map.permission :edit_time_entries, {:timelog => [:edit, :destroy]}, :require => :member
     map.permission :edit_own_time_entries, {:timelog => [:edit, :destroy]}, :require => :loggedin
     map.permission :manage_project_activities, {:project_enumerations => [:update, :destroy]}, :require => :member

test/functional/time_entry_reports_controller_test.rb

@@ -0,0 +1,134 @@
+# -*- coding: utf-8 -*-
+require File.dirname(__FILE__) + '/../test_helper'
+
+class TimeEntryReportsControllerTest < ActionController::TestCase
+  fixtures :projects, :enabled_modules, :roles, :members, :member_roles, :issues, :time_entries, :users, :trackers, :enumerations, :issue_statuses, :custom_fields, :custom_values
+
+  def test_report_no_criteria
+    get :report, :project_id => 1
+    assert_response :success
+    assert_template 'report'
+  end
+  
+  def test_report_all_projects
+    get :report
+    assert_response :success
+    assert_template 'report'
+  end
+  
+  def test_report_all_projects_denied
+    r = Role.anonymous
+    r.permissions.delete(:view_time_entries)
+    r.permissions_will_change!
+    r.save
+    get :report
+    assert_redirected_to '/login?back_url=http%3A%2F%2Ftest.host%2Ftime_entries%2Freport'
+  end
+  
+  def test_report_all_projects_one_criteria
+    get :report, :columns => 'week', :from => "2007-04-01", :to => "2007-04-30", :criterias => ['project']
+    assert_response :success
+    assert_template 'report'
+    assert_not_nil assigns(:total_hours)
+    assert_equal "8.65", "%.2f" % assigns(:total_hours)
+  end
+
+  def test_report_all_time
+    get :report, :project_id => 1, :criterias => ['project', 'issue']
+    assert_response :success
+    assert_template 'report'
+    assert_not_nil assigns(:total_hours)
+    assert_equal "162.90", "%.2f" % assigns(:total_hours)
+  end
+
+  def test_report_all_time_by_day
+    get :report, :project_id => 1, :criterias => ['project', 'issue'], :columns => 'day'
+    assert_response :success
+    assert_template 'report'
+    assert_not_nil assigns(:total_hours)
+    assert_equal "162.90", "%.2f" % assigns(:total_hours)
+    assert_tag :tag => 'th', :content => '2007-03-12'
+  end
+  
+  def test_report_one_criteria
+    get :report, :project_id => 1, :columns => 'week', :from => "2007-04-01", :to => "2007-04-30", :criterias => ['project']
+    assert_response :success
+    assert_template 'report'
+    assert_not_nil assigns(:total_hours)
+    assert_equal "8.65", "%.2f" % assigns(:total_hours)
+  end
+  
+  def test_report_two_criterias
+    get :report, :project_id => 1, :columns => 'month', :from => "2007-01-01", :to => "2007-12-31", :criterias => ["member", "activity"]
+    assert_response :success
+    assert_template 'report'
+    assert_not_nil assigns(:total_hours)
+    assert_equal "162.90", "%.2f" % assigns(:total_hours)
+  end
+  
+  def test_report_one_day
+    get :report, :project_id => 1, :columns => 'day', :from => "2007-03-23", :to => "2007-03-23", :criterias => ["member", "activity"]
+    assert_response :success
+    assert_template 'report'
+    assert_not_nil assigns(:total_hours)
+    assert_equal "4.25", "%.2f" % assigns(:total_hours)
+  end
+  
+  def test_report_at_issue_level
+    get :report, :project_id => 1, :issue_id => 1, :columns => 'month', :from => "2007-01-01", :to => "2007-12-31", :criterias => ["member", "activity"]
+    assert_response :success
+    assert_template 'report'
+    assert_not_nil assigns(:total_hours)
+    assert_equal "154.25", "%.2f" % assigns(:total_hours)
+  end
+  
+  def test_report_custom_field_criteria
+    get :report, :project_id => 1, :criterias => ['project', 'cf_1', 'cf_7']
+    assert_response :success
+    assert_template 'report'
+    assert_not_nil assigns(:total_hours)
+    assert_not_nil assigns(:criterias)
+    assert_equal 3, assigns(:criterias).size
+    assert_equal "162.90", "%.2f" % assigns(:total_hours)
+    # Custom field column
+    assert_tag :tag => 'th', :content => 'Database'
+    # Custom field row
+    assert_tag :tag => 'td', :content => 'MySQL',
+                             :sibling => { :tag => 'td', :attributes => { :class => 'hours' },
+                                                         :child => { :tag => 'span', :attributes => { :class => 'hours hours-int' },
+                                                                                     :content => '1' }}
+    # Second custom field column
+    assert_tag :tag => 'th', :content => 'Billable'
+  end
+  
+  def test_report_one_criteria_no_result
+    get :report, :project_id => 1, :columns => 'week', :from => "1998-04-01", :to => "1998-04-30", :criterias => ['project']
+    assert_response :success
+    assert_template 'report'
+    assert_not_nil assigns(:total_hours)
+    assert_equal "0.00", "%.2f" % assigns(:total_hours)
+  end
+  
+  def test_report_all_projects_csv_export
+    get :report, :columns => 'month', :from => "2007-01-01", :to => "2007-06-30", :criterias => ["project", "member", "activity"], :format => "csv"
+    assert_response :success
+    assert_equal 'text/csv', @response.content_type
+    lines = @response.body.chomp.split("\n")
+    # Headers
+    assert_equal 'Project,Member,Activity,2007-1,2007-2,2007-3,2007-4,2007-5,2007-6,Total', lines.first
+    # Total row
+    assert_equal 'Total,"","","","",154.25,8.65,"","",162.90', lines.last
+  end
+  
+  def test_report_csv_export
+    get :report, :project_id => 1, :columns => 'month', :from => "2007-01-01", :to => "2007-06-30", :criterias => ["project", "member", "activity"], :format => "csv"
+    assert_response :success
+    assert_equal 'text/csv', @response.content_type
+    lines = @response.body.chomp.split("\n")
+    # Headers
+    assert_equal 'Project,Member,Activity,2007-1,2007-2,2007-3,2007-4,2007-5,2007-6,Total', lines.first
+    # Total row
+    assert_equal 'Total,"","","","",154.25,8.65,"","",162.90', lines.last
+  end
+  
+end

test/functional/timelog_controller_test.rb

@@ -137,133 +137,6 @@ class TimelogControllerTest < ActionController::TestCase
     TimeEntry.before_destroy.reject! {|callback| callback.method == :stop_callback_chain }
   end
   
-  def test_report_no_criteria
-    get :report, :project_id => 1
-    assert_response :success
-    assert_template 'report'
-  end
-  
-  def test_report_all_projects
-    get :report
-    assert_response :success
-    assert_template 'report'
-  end
-  
-  def test_report_all_projects_denied
-    r = Role.anonymous
-    r.permissions.delete(:view_time_entries)
-    r.permissions_will_change!
-    r.save
-    get :report
-    assert_redirected_to '/login?back_url=http%3A%2F%2Ftest.host%2Ftime_entries%2Freport'
-  end
-  
-  def test_report_all_projects_one_criteria
-    get :report, :columns => 'week', :from => "2007-04-01", :to => "2007-04-30", :criterias => ['project']
-    assert_response :success
-    assert_template 'report'
-    assert_not_nil assigns(:total_hours)
-    assert_equal "8.65", "%.2f" % assigns(:total_hours)
-  end
-
-  def test_report_all_time
-    get :report, :project_id => 1, :criterias => ['project', 'issue']
-    assert_response :success
-    assert_template 'report'
-    assert_not_nil assigns(:total_hours)
-    assert_equal "162.90", "%.2f" % assigns(:total_hours)
-  end
-
-  def test_report_all_time_by_day
-    get :report, :project_id => 1, :criterias => ['project', 'issue'], :columns => 'day'
-    assert_response :success
-    assert_template 'report'
-    assert_not_nil assigns(:total_hours)
-    assert_equal "162.90", "%.2f" % assigns(:total_hours)
-    assert_tag :tag => 'th', :content => '2007-03-12'
-  end
-  
-  def test_report_one_criteria
-    get :report, :project_id => 1, :columns => 'week', :from => "2007-04-01", :to => "2007-04-30", :criterias => ['project']
-    assert_response :success
-    assert_template 'report'
-    assert_not_nil assigns(:total_hours)
-    assert_equal "8.65", "%.2f" % assigns(:total_hours)
-  end
-  
-  def test_report_two_criterias
-    get :report, :project_id => 1, :columns => 'month', :from => "2007-01-01", :to => "2007-12-31", :criterias => ["member", "activity"]
-    assert_response :success
-    assert_template 'report'
-    assert_not_nil assigns(:total_hours)
-    assert_equal "162.90", "%.2f" % assigns(:total_hours)
-  end
-  
-  def test_report_one_day
-    get :report, :project_id => 1, :columns => 'day', :from => "2007-03-23", :to => "2007-03-23", :criterias => ["member", "activity"]
-    assert_response :success
-    assert_template 'report'
-    assert_not_nil assigns(:total_hours)
-    assert_equal "4.25", "%.2f" % assigns(:total_hours)
-  end
-  
-  def test_report_at_issue_level
-    get :report, :project_id => 1, :issue_id => 1, :columns => 'month', :from => "2007-01-01", :to => "2007-12-31", :criterias => ["member", "activity"]
-    assert_response :success
-    assert_template 'report'
-    assert_not_nil assigns(:total_hours)
-    assert_equal "154.25", "%.2f" % assigns(:total_hours)
-  end
-  
-  def test_report_custom_field_criteria
-    get :report, :project_id => 1, :criterias => ['project', 'cf_1', 'cf_7']
-    assert_response :success
-    assert_template 'report'
-    assert_not_nil assigns(:total_hours)
-    assert_not_nil assigns(:criterias)
-    assert_equal 3, assigns(:criterias).size
-    assert_equal "162.90", "%.2f" % assigns(:total_hours)
-    # Custom field column
-    assert_tag :tag => 'th', :content => 'Database'
-    # Custom field row
-    assert_tag :tag => 'td', :content => 'MySQL',
-                             :sibling => { :tag => 'td', :attributes => { :class => 'hours' },
-                                                         :child => { :tag => 'span', :attributes => { :class => 'hours hours-int' },
-                                                                                     :content => '1' }}
-    # Second custom field column
-    assert_tag :tag => 'th', :content => 'Billable'
-  end
-  
-  def test_report_one_criteria_no_result
-    get :report, :project_id => 1, :columns => 'week', :from => "1998-04-01", :to => "1998-04-30", :criterias => ['project']
-    assert_response :success
-    assert_template 'report'
-    assert_not_nil assigns(:total_hours)
-    assert_equal "0.00", "%.2f" % assigns(:total_hours)
-  end
-  
-  def test_report_all_projects_csv_export
-    get :report, :columns => 'month', :from => "2007-01-01", :to => "2007-06-30", :criterias => ["project", "member", "activity"], :format => "csv"
-    assert_response :success
-    assert_equal 'text/csv', @response.content_type
-    lines = @response.body.chomp.split("\n")
-    # Headers
-    assert_equal 'Project,Member,Activity,2007-1,2007-2,2007-3,2007-4,2007-5,2007-6,Total', lines.first
-    # Total row
-    assert_equal 'Total,"","","","",154.25,8.65,"","",162.90', lines.last
-  end
-  
-  def test_report_csv_export
-    get :report, :project_id => 1, :columns => 'month', :from => "2007-01-01", :to => "2007-06-30", :criterias => ["project", "member", "activity"], :format => "csv"
-    assert_response :success
-    assert_equal 'text/csv', @response.content_type
-    lines = @response.body.chomp.split("\n")
-    # Headers
-    assert_equal 'Project,Member,Activity,2007-1,2007-2,2007-3,2007-4,2007-5,2007-6,Total', lines.first
-    # Total row
-    assert_equal 'Total,"","","","",154.25,8.65,"","",162.90', lines.last
-  end
-  
   def test_details_all_projects
     get :details
     assert_response :success

test/integration/routing_test.rb

@@ -226,9 +226,15 @@ class RoutingTest < ActionController::IntegrationTest
     should_route :get, "/projects/ecookbook/time_entries/new", :controller => 'timelog', :action => 'edit', :project_id => 'ecookbook'
     should_route :get, "/projects/ecookbook/issues/567/time_entries/new", :controller => 'timelog', :action => 'edit', :project_id => 'ecookbook', :issue_id => '567'
     should_route :get, "/time_entries/22/edit", :controller => 'timelog', :action => 'edit', :id => '22'
-    should_route :get, "/time_entries/report", :controller => 'timelog', :action => 'report'
-    should_route :get, "/projects/567/time_entries/report", :controller => 'timelog', :action => 'report', :project_id => '567'
-    should_route :get, "/projects/567/time_entries/report.csv", :controller => 'timelog', :action => 'report', :project_id => '567', :format => 'csv'
+
+    should_route :post, "/time_entries/55/destroy", :controller => 'timelog', :action => 'destroy', :id => '55'
+  end
+
+  context "time_entry_reports" do
+    should_route :get, "/time_entries/report", :controller => 'time_entry_reports', :action => 'report'
+    should_route :get, "/projects/567/time_entries/report", :controller => 'time_entry_reports', :action => 'report', :project_id => '567'
+    should_route :get, "/projects/567/time_entries/report.csv", :controller => 'time_entry_reports', :action => 'report', :project_id => '567', :format => 'csv'
+    
     should_route :get, "/time_entries", :controller => 'timelog', :action => 'details'
     should_route :get, "/time_entries.csv", :controller => 'timelog', :action => 'details', :format => 'csv'
     should_route :get, "/time_entries.atom", :controller => 'timelog', :action => 'details', :format => 'atom'
@@ -240,7 +246,6 @@ class RoutingTest < ActionController::IntegrationTest
     should_route :get, "/issues/234/time_entries.atom", :controller => 'timelog', :action => 'details', :issue_id => '234', :format => 'atom'
     should_route :get, "/projects/ecookbook/issues/123/time_entries", :controller => 'timelog', :action => 'details', :project_id => 'ecookbook', :issue_id => '123'
 
-    should_route :post, "/time_entries/55/destroy", :controller => 'timelog', :action => 'destroy', :id => '55'
   end
 
   context "users" do