From beb5e6039166dadc8efccb65feb0f7ee450c9558 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Tue, 12 Jul 2016 17:40:19 +0000 Subject: [PATCH] Makes spent time queries savable (#14790). git-svn-id: http://svn.redmine.org/redmine/trunk@15639 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/queries_controller.rb | 17 ++++++++++---- app/controllers/timelog_controller.rb | 9 +++++--- app/helpers/queries_helper.rb | 21 +++++++++-------- app/views/queries/new.html.erb | 1 + app/views/timelog/_date_range.html.erb | 17 +++++++++++++- app/views/timelog/index.html.erb | 4 ++-- app/views/timelog/report.html.erb | 4 ++-- lib/redmine/field_format.rb | 2 +- test/functional/issues_controller_test.rb | 14 +++++------ test/functional/queries_controller_test.rb | 27 ++++++++++++++++++++++ test/functional/timelog_controller_test.rb | 10 ++++++++ 11 files changed, 96 insertions(+), 30 deletions(-) diff --git a/app/controllers/queries_controller.rb b/app/controllers/queries_controller.rb index 961b75dcc..84843c1dd 100644 --- a/app/controllers/queries_controller.rb +++ b/app/controllers/queries_controller.rb @@ -60,7 +60,7 @@ class QueriesController < ApplicationController if @query.save flash[:notice] = l(:notice_successful_create) - redirect_to_issues(:query_id => @query) + redirect_to_items(:query_id => @query) else render :action => 'new', :layout => !request.xhr? end @@ -74,7 +74,7 @@ class QueriesController < ApplicationController if @query.save flash[:notice] = l(:notice_successful_update) - redirect_to_issues(:query_id => @query) + redirect_to_items(:query_id => @query) else render :action => 'edit' end @@ -82,7 +82,7 @@ class QueriesController < ApplicationController def destroy @query.destroy - redirect_to_issues(:set_filter => 1) + redirect_to_items(:set_filter => 1) end private @@ -117,7 +117,12 @@ class QueriesController < ApplicationController @query end - def redirect_to_issues(options) + def redirect_to_items(options) + method = "redirect_to_#{@query.class.name.underscore}" + send method, options + end + + def redirect_to_issue_query(options) if params[:gantt] if @project redirect_to project_gantt_path(@project, options) @@ -129,6 +134,10 @@ class QueriesController < ApplicationController end end + def redirect_to_time_entry_query(options) + redirect_to _time_entries_path(@project, nil, options) + end + # Returns the Query subclass, IssueQuery by default # for compatibility with previous behaviour def query_class diff --git a/app/controllers/timelog_controller.rb b/app/controllers/timelog_controller.rb index 66ae97678..a0f0ea31d 100644 --- a/app/controllers/timelog_controller.rb +++ b/app/controllers/timelog_controller.rb @@ -40,8 +40,7 @@ class TimelogController < ApplicationController include QueriesHelper def index - @query = TimeEntryQuery.build_from_params(params, :project => @project, :name => '_') - + retrieve_time_entry_query sort_init(@query.sort_criteria.empty? ? [['spent_on', 'desc']] : @query.sort_criteria) sort_update(@query.sortable_columns) scope = time_entry_scope(:order => sort_clause). @@ -75,7 +74,7 @@ class TimelogController < ApplicationController end def report - @query = TimeEntryQuery.build_from_params(params, :project => @project, :name => '_') + retrieve_time_entry_query scope = time_entry_scope @report = Redmine::Helpers::TimeReport.new(@project, @issue, params[:criteria], params[:columns], scope) @@ -271,4 +270,8 @@ private end scope end + + def retrieve_time_entry_query + retrieve_query(TimeEntryQuery, false) + end end diff --git a/app/helpers/queries_helper.rb b/app/helpers/queries_helper.rb index 0bea000db..f24e51327 100644 --- a/app/helpers/queries_helper.rb +++ b/app/helpers/queries_helper.rb @@ -204,26 +204,27 @@ module QueriesHelper end # Retrieve query from session or build a new query - def retrieve_query - if !params[:query_id].blank? + def retrieve_query(klass=IssueQuery, use_session=true) + session_key = klass.name.underscore.to_sym + + if params[:query_id].present? cond = "project_id IS NULL" cond << " OR project_id = #{@project.id}" if @project - @query = IssueQuery.where(cond).find(params[:query_id]) + @query = klass.where(cond).find(params[:query_id]) raise ::Unauthorized unless @query.visible? @query.project = @project - session[:query] = {:id => @query.id, :project_id => @query.project_id} + session[session_key] = {:id => @query.id, :project_id => @query.project_id} if use_session sort_clear - elsif api_request? || params[:set_filter] || session[:query].nil? || session[:query][:project_id] != (@project ? @project.id : nil) + elsif api_request? || params[:set_filter] || !use_session || session[session_key].nil? || session[session_key][:project_id] != (@project ? @project.id : nil) # Give it a name, required to be valid - @query = IssueQuery.new(:name => "_") - @query.project = @project + @query = klass.new(:name => "_", :project => @project) @query.build_from_params(params) - session[:query] = {:project_id => @query.project_id, :filters => @query.filters, :group_by => @query.group_by, :column_names => @query.column_names, :totalable_names => @query.totalable_names} + session[session_key] = {:project_id => @query.project_id, :filters => @query.filters, :group_by => @query.group_by, :column_names => @query.column_names, :totalable_names => @query.totalable_names} if use_session else # retrieve from session @query = nil - @query = IssueQuery.find_by_id(session[:query][:id]) if session[:query][:id] - @query ||= IssueQuery.new(:name => "_", :filters => session[:query][:filters], :group_by => session[:query][:group_by], :column_names => session[:query][:column_names], :totalable_names => session[:query][:totalable_names]) + @query = klass.find_by_id(session[session_key][:id]) if session[session_key][:id] + @query ||= klass.new(:name => "_", :filters => session[session_key][:filters], :group_by => session[session_key][:group_by], :column_names => session[session_key][:column_names], :totalable_names => session[session_key][:totalable_names]) @query.project = @project end end diff --git a/app/views/queries/new.html.erb b/app/views/queries/new.html.erb index 22cfe7331..6df43dba2 100644 --- a/app/views/queries/new.html.erb +++ b/app/views/queries/new.html.erb @@ -1,6 +1,7 @@

<%= l(:label_query_new) %>

<%= form_tag(@project ? project_queries_path(@project) : queries_path, :id => "query-form") do %> + <%= hidden_field_tag 'type', @query.class.name %> <%= render :partial => 'form', :locals => {:query => @query} %> <%= submit_tag l(:button_save) %> <% end %> diff --git a/app/views/timelog/_date_range.html.erb b/app/views/timelog/_date_range.html.erb index 9e6014653..561f473dc 100644 --- a/app/views/timelog/_date_range.html.erb +++ b/app/views/timelog/_date_range.html.erb @@ -1,4 +1,5 @@
+<%= hidden_field_tag 'set_filter', '1' %>
"> <%= l(:label_filter_plural) %> @@ -22,11 +23,25 @@

<%= link_to_function l(:button_apply), '$("#query_form").submit()', :class => 'icon icon-checked' %> <%= link_to l(:button_clear), {:project_id => @project, :issue_id => @issue}, :class => 'icon icon-reload' %> + <% if @query.new_record? %> + <% if User.current.allowed_to?(:save_queries, @project, :global => true) %> + <%= link_to_function l(:button_save), + "$('#query_form').attr('action', '#{ @project ? new_project_query_path(@project) : new_query_path }').submit()", + :class => 'icon icon-save' %> + <% end %> + <% else %> + <% if @query.editable_by?(User.current) %> + <%= link_to l(:button_edit), edit_query_path(@query), :class => 'icon icon-edit' %> + <%= delete_link query_path(@query) %> + <% end %> + <% end %>

+ +<%= hidden_field_tag 'type', 'TimeEntryQuery' %>
-<% query_params = params.slice(:f, :op, :v, :sort) %> +<% query_params = params.slice(:f, :op, :v, :sort, :query_id) %>
  • <%= link_to(l(:label_details), _time_entries_path(@project, @issue, query_params), :class => (action_name == 'index' ? 'selected' : nil)) %>
    • diff --git a/app/views/timelog/index.html.erb b/app/views/timelog/index.html.erb index b32e7092c..6f2c92f4e 100644 --- a/app/views/timelog/index.html.erb +++ b/app/views/timelog/index.html.erb @@ -6,7 +6,7 @@ <%= render_timelog_breadcrumb %> -

    <%= l(:label_spent_time) %>

    +

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

    <%= form_tag(params.slice(:project_id, :issue_id), :method => :get, :id => 'query_form') do %> <%= render :partial => 'date_range' %> @@ -41,7 +41,7 @@
<% end %> -<% html_title l(:label_spent_time), l(:label_details) %> +<% html_title(@query.new_record? ? l(:label_spent_time) : @query.name, l(:label_details)) %> <% content_for :header_tags do %> <%= auto_discovery_link_tag(:atom, {:issue_id => @issue, :format => 'atom', :key => User.current.rss_key}, :title => l(:label_spent_time)) %> diff --git a/app/views/timelog/report.html.erb b/app/views/timelog/report.html.erb index 176f128e0..d1d215542 100644 --- a/app/views/timelog/report.html.erb +++ b/app/views/timelog/report.html.erb @@ -6,7 +6,7 @@ <%= render_timelog_breadcrumb %> -

<%= l(:label_spent_time) %>

+

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

<%= form_tag(params.slice(:project_id, :issue_id), :method => :get, :id => 'query_form') do %> <% @report.criteria.each do |criterion| %> @@ -70,5 +70,5 @@ <% end %> <% end %> -<% html_title l(:label_spent_time), l(:label_report) %> +<% html_title(@query.new_record? ? l(:label_spent_time) : @query.name, l(:label_report)) %> diff --git a/lib/redmine/field_format.rb b/lib/redmine/field_format.rb index 0347ca8da..3385989ac 100644 --- a/lib/redmine/field_format.rb +++ b/lib/redmine/field_format.rb @@ -806,7 +806,7 @@ module Redmine scope = object.project.shared_versions filtered_versions_options(custom_field, scope, all_statuses) elsif object.nil? - scope = Version.visible.where(:sharing => 'system') + scope = ::Version.visible.where(:sharing => 'system') filtered_versions_options(custom_field, scope, all_statuses) else [] diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index 176fd42e5..d571d4bda 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -399,9 +399,9 @@ class IssuesControllerTest < ActionController::TestCase def test_index_with_query_id_and_project_id_should_set_session_query get :index, :project_id => 1, :query_id => 4 assert_response :success - assert_kind_of Hash, session[:query] - assert_equal 4, session[:query][:id] - assert_equal 1, session[:query][:project_id] + assert_kind_of Hash, session[:issue_query] + assert_equal 4, session[:issue_query][:id] + assert_equal 1, session[:issue_query][:project_id] end def test_index_with_invalid_query_id_should_respond_404 @@ -411,7 +411,7 @@ class IssuesControllerTest < ActionController::TestCase def test_index_with_cross_project_query_in_session_should_show_project_issues q = IssueQuery.create!(:name => "test", :user_id => 2, :visibility => IssueQuery::VISIBILITY_PRIVATE, :project => nil) - @request.session[:query] = {:id => q.id, :project_id => 1} + @request.session[:issue_query] = {:id => q.id, :project_id => 1} with_settings :display_subprojects_issues => '0' do get :index, :project_id => 1 @@ -832,9 +832,9 @@ class IssuesControllerTest < ActionController::TestCase assert_equal columns, query.column_names.map(&:to_s) # columns should be stored in session - assert_kind_of Hash, session[:query] - assert_kind_of Array, session[:query][:column_names] - assert_equal columns, session[:query][:column_names].map(&:to_s) + assert_kind_of Hash, session[:issue_query] + assert_kind_of Array, session[:issue_query][:column_names] + assert_equal columns, session[:issue_query][:column_names].map(&:to_s) # ensure only these columns are kept in the selected columns list assert_select 'select#selected_columns option' do diff --git a/test/functional/queries_controller_test.rb b/test/functional/queries_controller_test.rb index c7ad25850..27eb48a92 100644 --- a/test/functional/queries_controller_test.rb +++ b/test/functional/queries_controller_test.rb @@ -58,6 +58,13 @@ class QueriesControllerTest < ActionController::TestCase assert_response 404 end + def test_new_time_entry_query + @request.session[:user_id] = 2 + get :new, :project_id => 1, :type => 'TimeEntryQuery' + assert_response :success + assert_select 'input[name=type][value=?]', 'TimeEntryQuery' + end + def test_create_project_public_query @request.session[:user_id] = 2 post :create, @@ -263,6 +270,26 @@ class QueriesControllerTest < ActionController::TestCase assert_equal Query::VISIBILITY_PUBLIC, query.visibility end + def test_create_project_public_time_entry_query + @request.session[:user_id] = 2 + + q = new_record(TimeEntryQuery) do + post :create, + :project_id => 'ecookbook', + :type => 'TimeEntryQuery', + :default_columns => '1', + :f => ["spent_on"], + :op => {"spent_on" => "="}, + :v => { "spent_on" => ["2016-07-14"]}, + :query => {"name" => "test_new_project_public_query", "visibility" => "2"} + end + + assert_redirected_to :controller => 'timelog', :action => 'index', :project_id => 'ecookbook', :query_id => q.id + assert q.is_public? + assert q.has_default_columns? + assert q.valid? + end + def test_edit_global_public_query @request.session[:user_id] = 1 get :edit, :id => 4 diff --git a/test/functional/timelog_controller_test.rb b/test/functional/timelog_controller_test.rb index c3e96793d..5ee683f4c 100644 --- a/test/functional/timelog_controller_test.rb +++ b/test/functional/timelog_controller_test.rb @@ -759,6 +759,16 @@ class TimelogControllerTest < ActionController::TestCase assert_equal values.sort, values end + def test_index_with_query + query = TimeEntryQuery.new(:project_id => 1, :name => 'Time Entry Query', :visibility => 2) + query.save! + @request.session[:user_id] = 2 + + get :index, :project_id => 'ecookbook', :query_id => query.id + assert_response :success + assert_select 'h2', :text => query.name + end + def test_index_atom_feed get :index, :project_id => 1, :format => 'atom' assert_response :success -- 2.39.5