From: Jean-Philippe Lang Date: Tue, 12 Jul 2016 19:28:49 +0000 (+0000) Subject: Remove special behaviour for listing issue time entries, use a filter for that. X-Git-Tag: 3.4.0~819 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=03dbf8abb881e333256b1cf046b04532b3164d21;p=redmine.git Remove special behaviour for listing issue time entries, use a filter for that. git-svn-id: http://svn.redmine.org/redmine/trunk@15644 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- diff --git a/app/controllers/timelog_controller.rb b/app/controllers/timelog_controller.rb index a0f0ea31d..a63ffae82 100644 --- a/app/controllers/timelog_controller.rb +++ b/app/controllers/timelog_controller.rb @@ -22,7 +22,8 @@ class TimelogController < ApplicationController before_filter :find_time_entries, :only => [:bulk_edit, :bulk_update, :destroy] before_filter :authorize, :only => [:show, :edit, :update, :bulk_edit, :bulk_update, :destroy] - before_filter :find_optional_project, :only => [:new, :create, :index, :report] + before_filter :find_optional_issue, :only => [:new, :create] + before_filter :find_optional_project, :only => [:index, :report] before_filter :authorize_global, :only => [:new, :create, :index, :report] accept_rss_auth :index @@ -251,11 +252,17 @@ private end end - def find_optional_project + def find_optional_issue if params[:issue_id].present? @issue = Issue.find(params[:issue_id]) @project = @issue.project - elsif params[:project_id].present? + else + find_optional_project + end + end + + def find_optional_project + if params[:project_id].present? @project = Project.find(params[:project_id]) end rescue ActiveRecord::RecordNotFound @@ -264,11 +271,7 @@ private # Returns the TimeEntry scope for index and report actions def time_entry_scope(options={}) - scope = @query.results_scope(options) - if @issue - scope = scope.on_issue(@issue) - end - scope + @query.results_scope(options) end def retrieve_time_entry_query diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index 2557b8cc7..8d23d95cd 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -135,11 +135,13 @@ module IssuesHelper def issue_spent_hours_details(issue) if issue.total_spent_hours > 0 + path = project_time_entries_path(issue.project, :issue_id => "~#{issue.id}") + if issue.total_spent_hours == issue.spent_hours - link_to(l_hours_short(issue.spent_hours), issue_time_entries_path(issue)) + link_to(l_hours_short(issue.spent_hours), path) else s = issue.spent_hours > 0 ? l_hours_short(issue.spent_hours) : "" - s << " (#{l(:label_total)}: #{link_to l_hours_short(issue.total_spent_hours), issue_time_entries_path(issue)})" + s << " (#{l(:label_total)}: #{link_to l_hours_short(issue.total_spent_hours), path})" s.html_safe end end diff --git a/app/helpers/queries_helper.rb b/app/helpers/queries_helper.rb index d457593d5..e2e09c7de 100644 --- a/app/helpers/queries_helper.rb +++ b/app/helpers/queries_helper.rb @@ -24,8 +24,10 @@ module QueriesHelper ungrouped = [] grouped = {} query.available_filters.map do |field, field_options| - if [:tree, :relation].include?(field_options[:type]) + if field_options[:type] == :relation group = :label_relations + elsif field_options[:type] == :tree + group = query.is_a?(IssueQuery) ? :label_relations : nil elsif field =~ /^(.+)\./ # association filters group = "field_#{$1}" diff --git a/app/helpers/routes_helper.rb b/app/helpers/routes_helper.rb index 85d60a28a..a410224a7 100644 --- a/app/helpers/routes_helper.rb +++ b/app/helpers/routes_helper.rb @@ -54,9 +54,7 @@ module RoutesHelper end def _time_entries_path(project, issue, *args) - if issue - issue_time_entries_path(issue, *args) - elsif project + if project project_time_entries_path(project, *args) else time_entries_path(*args) @@ -64,9 +62,7 @@ module RoutesHelper end def _report_time_entries_path(project, issue, *args) - if issue - report_issue_time_entries_path(issue, *args) - elsif project + if project report_project_time_entries_path(project, *args) else report_time_entries_path(*args) diff --git a/app/helpers/timelog_helper.rb b/app/helpers/timelog_helper.rb index 25bc750f3..8f9d47d74 100644 --- a/app/helpers/timelog_helper.rb +++ b/app/helpers/timelog_helper.rb @@ -20,20 +20,6 @@ module TimelogHelper include ApplicationHelper - def render_timelog_breadcrumb - links = [] - links << link_to(l(:label_project_all), {:project_id => nil, :issue_id => nil}) - links << link_to(h(@project), {:project_id => @project, :issue_id => nil}) if @project - if @issue - if @issue.visible? - links << link_to_issue(@issue, :subject => false) - else - links << "##{@issue.id}" - end - end - breadcrumb links - end - # Returns a collection of activities for a select field. time_entry # is optional and will be used to check if the selected TimeEntryActivity # is active. diff --git a/app/models/query.rb b/app/models/query.rb index 22f1fd898..1038637f2 100644 --- a/app/models/query.rb +++ b/app/models/query.rb @@ -460,7 +460,7 @@ class Query < ActiveRecord::Base next unless expression =~ /^#{Regexp.escape(operator)}(.*)$/ values = $1 add_filter field, operator, values.present? ? values.split('|') : [''] - end || add_filter(field, '=', expression.split('|')) + end || add_filter(field, '=', expression.to_s.split('|')) end # Add multiple filters using +add_filter+ diff --git a/app/models/time_entry_query.rb b/app/models/time_entry_query.rb index 99a52885e..bb042ff16 100644 --- a/app/models/time_entry_query.rb +++ b/app/models/time_entry_query.rb @@ -67,6 +67,9 @@ class TimeEntryQuery < Query ) unless project_values.empty? end end + + add_available_filter("issue_id", :type => :tree, :label => :label_issue) + principals.uniq! principals.sort! users = principals.select {|p| p.is_a?(User)} @@ -114,6 +117,24 @@ class TimeEntryQuery < Query includes(:activity). references(:activity) end + + def sql_for_issue_id_field(field, operator, value) + case operator + when "=" + "#{TimeEntry.table_name}.issue_id = #{value.first.to_i}" + when "~" + issue = Issue.where(:id => value.first.to_i).first + if issue && (issue_ids = issue.self_and_descendants.pluck(:id)).any? + "#{TimeEntry.table_name}.issue_id IN (#{issue_ids.join(',')})" + else + "1=0" + end + when "!*" + "#{TimeEntry.table_name}.issue_id IS NULL" + when "*" + "#{TimeEntry.table_name}.issue_id IS NOT NULL" + end + end def sql_for_activity_id_field(field, operator, value) condition_on_id = sql_for_field(field, operator, value, Enumeration.table_name, 'id') diff --git a/app/views/timelog/_date_range.html.erb b/app/views/timelog/_date_range.html.erb index 561f473dc..91b96d8b7 100644 --- a/app/views/timelog/_date_range.html.erb +++ b/app/views/timelog/_date_range.html.erb @@ -41,11 +41,11 @@
-<% query_params = params.slice(:f, :op, :v, :sort, :query_id) %> +<% query_params = request.query_parameters %>
diff --git a/app/views/timelog/index.html.erb b/app/views/timelog/index.html.erb index 02d5dbf9b..d6ea9690e 100644 --- a/app/views/timelog/index.html.erb +++ b/app/views/timelog/index.html.erb @@ -4,11 +4,9 @@ :class => 'icon icon-time-add' if User.current.allowed_to?(:log_time, @project, :global => true) %> -<%= render_timelog_breadcrumb %> -

<%= @query.new_record? ? l(:label_spent_time) : @query.name %>

-<%= form_tag(params.slice(:project_id, :issue_id), :method => :get, :id => 'query_form') do %> +<%= form_tag(_time_entries_path(@project, nil), :method => :get, :id => 'query_form') do %> <%= render :partial => 'date_range' %> <% end %> diff --git a/app/views/timelog/report.html.erb b/app/views/timelog/report.html.erb index f3cd6dc60..efa04de88 100644 --- a/app/views/timelog/report.html.erb +++ b/app/views/timelog/report.html.erb @@ -4,11 +4,9 @@ :class => 'icon icon-time-add' if User.current.allowed_to?(:log_time, @project, :global => true) %> -<%= render_timelog_breadcrumb %> -

<%= @query.new_record? ? l(:label_spent_time) : @query.name %>

-<%= form_tag(params.slice(:project_id, :issue_id), :method => :get, :id => 'query_form') do %> +<%= form_tag(_report_time_entries_path(@project, nil), :method => :get, :id => 'query_form') do %> <% @report.criteria.each do |criterion| %> <%= hidden_field_tag 'criteria[]', criterion, :id => nil %> <% end %> diff --git a/config/routes.rb b/config/routes.rb index 1e49a9dd5..b8e465a63 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -188,11 +188,7 @@ Rails.application.routes.draw do match 'bulk_edit', :via => [:get, :post] post 'bulk_update' end - resources :time_entries, :controller => 'timelog', :except => [:show, :edit, :update, :destroy] do - collection do - get 'report' - end - end + resources :time_entries, :controller => 'timelog', :only => [:new, :create] shallow do resources :relations, :controller => 'issue_relations', :only => [:index, :show, :create, :destroy] end diff --git a/test/functional/time_entry_reports_controller_test.rb b/test/functional/time_entry_reports_controller_test.rb index a70beec4a..c4ba0f7fe 100644 --- a/test/functional/time_entry_reports_controller_test.rb +++ b/test/functional/time_entry_reports_controller_test.rb @@ -128,15 +128,6 @@ class TimeEntryReportsControllerTest < ActionController::TestCase assert_equal "4.25", "%.2f" % assigns(:report).total_hours end - def test_report_at_issue_level - get :report, :issue_id => 1, :columns => 'month', :from => "2007-01-01", :to => "2007-12-31", :criteria => ["user", "activity"] - assert_response :success - assert_template 'report' - assert_not_nil assigns(:report) - assert_equal "154.25", "%.2f" % assigns(:report).total_hours - assert_select 'form#query_form[action=?]', '/issues/1/time_entries/report' - end - def test_report_by_week_should_use_commercial_year TimeEntry.delete_all TimeEntry.generate!(:hours => '2', :spent_on => '2009-12-25') # 2009-52 diff --git a/test/functional/timelog_controller_test.rb b/test/functional/timelog_controller_test.rb index eafea27c7..25bfea2fa 100644 --- a/test/functional/timelog_controller_test.rb +++ b/test/functional/timelog_controller_test.rb @@ -676,20 +676,6 @@ class TimelogControllerTest < ActionController::TestCase assert_select 'form#query_form[action=?]', '/projects/ecookbook/time_entries' end - def test_index_at_issue_level - get :index, :issue_id => 1 - assert_response :success - assert_template 'index' - assert_not_nil assigns(:entries) - assert_equal 2, assigns(:entries).size - assert_not_nil assigns(:total_hours) - assert_equal 154.25, assigns(:total_hours) - # display all time - assert_nil assigns(:from) - assert_nil assigns(:to) - assert_select 'form#query_form[action=?]', '/issues/1/time_entries' - end - def test_index_should_sort_by_spent_on_and_created_on t1 = TimeEntry.create!(:user => User.find(1), :project => Project.find(1), :hours => 1, :spent_on => '2012-06-16', :created_on => '2012-06-16 20:00:00', :activity_id => 10) t2 = TimeEntry.create!(:user => User.find(1), :project => Project.find(1), :hours => 1, :spent_on => '2012-06-16', :created_on => '2012-06-16 20:05:00', :activity_id => 10) @@ -809,15 +795,6 @@ class TimelogControllerTest < ActionController::TestCase end end - def test_index_at_issue_level_should_include_csv_export_dialog - get :index, :issue_id => 3 - assert_response :success - - assert_select '#csv-export-options' do - assert_select 'form[action=?][method=get]', '/issues/3/time_entries.csv' - end - end - def test_index_csv_all_projects with_settings :date_format => '%m/%d/%Y' do get :index, :format => 'csv' diff --git a/test/integration/routing/timelog_test.rb b/test/integration/routing/timelog_test.rb index 9a2057329..b7a219c65 100644 --- a/test/integration/routing/timelog_test.rb +++ b/test/integration/routing/timelog_test.rb @@ -40,10 +40,7 @@ class RoutingTimelogsTest < Redmine::RoutingTest end def test_timelogs_scoped_under_issues - should_route 'GET /issues/234/time_entries' => 'timelog#index', :issue_id => '234' - should_route 'GET /issues/234/time_entries.csv' => 'timelog#index', :issue_id => '234', :format => 'csv' - should_route 'GET /issues/234/time_entries.atom' => 'timelog#index', :issue_id => '234', :format => 'atom' - should_route 'GET /issues/234/time_entries/new' => 'timelog#new', :issue_id => '234' + should_route 'GET /issues/234/time_entries/new' => 'timelog#new', :issue_id => '234' should_route 'POST /issues/234/time_entries' => 'timelog#create', :issue_id => '234' end @@ -53,9 +50,6 @@ class RoutingTimelogsTest < Redmine::RoutingTest should_route 'GET /projects/foo/time_entries/report' => 'timelog#report', :project_id => 'foo' should_route 'GET /projects/foo/time_entries/report.csv' => 'timelog#report', :project_id => 'foo', :format => 'csv' - - should_route 'GET /issues/234/time_entries/report' => 'timelog#report', :issue_id => '234' - should_route 'GET /issues/234/time_entries/report.csv' => 'timelog#report', :issue_id => '234', :format => 'csv' end def test_timelogs_bulk_edit diff --git a/test/unit/helpers/routes_helper_test.rb b/test/unit/helpers/routes_helper_test.rb index dd92ea67a..ec977c156 100644 --- a/test/unit/helpers/routes_helper_test.rb +++ b/test/unit/helpers/routes_helper_test.rb @@ -26,15 +26,11 @@ class RoutesHelperTest < ActionView::TestCase def test_time_entries_path assert_equal '/projects/ecookbook/time_entries', _time_entries_path(Project.find(1), nil) - assert_equal '/issues/1/time_entries', _time_entries_path(Project.find(1), Issue.find(1)) - assert_equal '/issues/1/time_entries', _time_entries_path(nil, Issue.find(1)) assert_equal '/time_entries', _time_entries_path(nil, nil) end def test_report_time_entries_path assert_equal '/projects/ecookbook/time_entries/report', _report_time_entries_path(Project.find(1), nil) - assert_equal '/issues/1/time_entries/report', _report_time_entries_path(Project.find(1), Issue.find(1)) - assert_equal '/issues/1/time_entries/report', _report_time_entries_path(nil, Issue.find(1)) assert_equal '/time_entries/report', _report_time_entries_path(nil, nil) end