From 41eab6615b31185831b110b5e3d29dc08cefceca Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sat, 21 Jan 2012 14:26:51 +0000 Subject: [PATCH] Enable global time logging at /time_entries/new (#10020). git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@8691 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/timelog_controller.rb | 19 +++-- app/models/project.rb | 11 +++ app/views/timelog/_form.html.erb | 7 ++ app/views/timelog/index.html.erb | 4 +- app/views/timelog/new.html.erb | 2 +- test/functional/timelog_controller_test.rb | 96 +++++++++++++++++++++- 6 files changed, 130 insertions(+), 9 deletions(-) diff --git a/app/controllers/timelog_controller.rb b/app/controllers/timelog_controller.rb index e1895f5de..ee15a6c8a 100644 --- a/app/controllers/timelog_controller.rb +++ b/app/controllers/timelog_controller.rb @@ -17,11 +17,15 @@ class TimelogController < ApplicationController menu_item :issues - before_filter :find_project, :only => [:new, :create] + + before_filter :find_project, :only => [:create] before_filter :find_time_entry, :only => [:show, :edit, :update] before_filter :find_time_entries, :only => [:bulk_edit, :bulk_update, :destroy] - before_filter :authorize, :except => [:index, :report] - before_filter :find_optional_project, :only => [:index, :report] + before_filter :authorize, :except => [:new, :index, :report] + + before_filter :find_optional_project, :only => [:new, :index, :report] + before_filter :authorize_global, :only => [:new, :index, :report] + accept_rss_auth :index accept_api_auth :index, :show, :create, :update, :destroy @@ -129,7 +133,11 @@ class TimelogController < ApplicationController format.html { flash[:notice] = l(:notice_successful_create) if params[:continue] - redirect_to :action => 'new', :project_id => @time_entry.project, :issue_id => @time_entry.issue + if params[:project_id] + redirect_to :action => 'new', :project_id => @time_entry.project, :issue_id => @time_entry.issue + else + redirect_to :action => 'new' + end else redirect_back_or_default :action => 'index', :project_id => @time_entry.project end @@ -138,7 +146,7 @@ class TimelogController < ApplicationController end else respond_to do |format| - format.html { render :action => 'edit' } + format.html { render :action => 'new' } format.api { render_validation_errors(@time_entry) } end end @@ -274,7 +282,6 @@ private 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 diff --git a/app/models/project.rb b/app/models/project.rb index 3760a646c..878b4c0ca 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -87,6 +87,17 @@ class Project < ActiveRecord::Base named_scope :status, lambda {|arg| arg.blank? ? {} : {:conditions => {:status => arg.to_i}} } named_scope :all_public, { :conditions => { :is_public => true } } named_scope :visible, lambda {|*args| {:conditions => Project.visible_condition(args.shift || User.current, *args) }} + named_scope :allowed_to, lambda {|*args| + user = User.current + permission = nil + if args.first.is_a?(Symbol) + permission = args.shift + else + user = args.shift + permission = args.shift + end + { :conditions => Project.allowed_to_condition(user, permission, *args) } + } named_scope :like, lambda {|arg| if arg.blank? {} diff --git a/app/views/timelog/_form.html.erb b/app/views/timelog/_form.html.erb index 9f7e28096..258de205f 100644 --- a/app/views/timelog/_form.html.erb +++ b/app/views/timelog/_form.html.erb @@ -2,6 +2,13 @@ <%= back_url_hidden_field_tag %>
+ <% if @time_entry.new_record? %> + <% if params[:project_id] %> + <%= f.hidden_field :project_id %> + <% else %> +

<%= f.select :project_id, project_tree_options_for_select(Project.allowed_to(:log_time).all, :selected => @time_entry.project), :required => true %>

+ <% end %> + <% end %>

<%= f.text_field :issue_id, :size => 6 %> <%= h("#{@time_entry.issue.tracker.name} ##{@time_entry.issue.id}: #{@time_entry.issue.subject}") if @time_entry.issue %>

<%= f.text_field :spent_on, :size => 10, :required => true %><%= calendar_for('time_entry_spent_on') %>

<%= f.text_field :hours, :size => 6, :required => true %>

diff --git a/app/views/timelog/index.html.erb b/app/views/timelog/index.html.erb index 55739412c..c3a63436e 100644 --- a/app/views/timelog/index.html.erb +++ b/app/views/timelog/index.html.erb @@ -1,5 +1,7 @@
-<%= link_to_if_authorized l(:button_log_time), {:controller => 'timelog', :action => 'new', :project_id => @project, :issue_id => @issue}, :class => 'icon icon-time-add' %> +<%= link_to l(:button_log_time), + {:controller => 'timelog', :action => 'new', :project_id => @project, :issue_id => @issue}, + :class => 'icon icon-time-add' if User.current.allowed_to?(:log_time, @project, :global => true) %>
<%= render_timelog_breadcrumb %> diff --git a/app/views/timelog/new.html.erb b/app/views/timelog/new.html.erb index 6871c5f23..f37574397 100644 --- a/app/views/timelog/new.html.erb +++ b/app/views/timelog/new.html.erb @@ -1,6 +1,6 @@

<%= l(:label_spent_time) %>

-<% labelled_form_for @time_entry, :url => project_time_entries_path(@time_entry.project) do |f| %> +<% labelled_form_for @time_entry, :url => time_entries_path do |f| %> <%= render :partial => 'form', :locals => {:f => f} %> <%= submit_tag l(:button_create) %> <%= submit_tag l(:button_create_and_continue), :name => 'continue' %> diff --git a/test/functional/timelog_controller_test.rb b/test/functional/timelog_controller_test.rb index 4dd8fe0c6..8a68a5dbf 100644 --- a/test/functional/timelog_controller_test.rb +++ b/test/functional/timelog_controller_test.rb @@ -51,7 +51,24 @@ class TimelogControllerTest < ActionController::TestCase get :new, :project_id => 1 assert_response :success assert_template 'new' - assert_no_tag :tag => 'option', :content => 'Inactive Activity' + assert_no_tag 'select', :attributes => {:name => 'time_entry[project_id]'} + assert_no_tag 'option', :content => 'Inactive Activity' + end + + def test_new_without_project + @request.session[:user_id] = 3 + get :new + assert_response :success + assert_template 'new' + assert_tag 'select', :attributes => {:name => 'time_entry[project_id]'} + end + + def test_new_without_project_should_deny_without_permission + Role.all.each {|role| role.remove_permission! :log_time} + @request.session[:user_id] = 3 + + get :new + assert_response 403 end def test_get_edit_existing_time @@ -141,6 +158,18 @@ class TimelogControllerTest < ActionController::TestCase assert_redirected_to '/projects/ecookbook/issues/1/time_entries/new' end + def test_create_and_continue_without_project + @request.session[:user_id] = 2 + post :create, :time_entry => {:project_id => '1', + :activity_id => '11', + :issue_id => '', + :spent_on => '2008-03-14', + :hours => '7.3'}, + :continue => '1' + + assert_redirected_to '/time_entries/new' + end + def test_create_without_log_time_permission_should_be_denied @request.session[:user_id] = 2 Role.find_by_name('Manager').remove_permission! :log_time @@ -153,6 +182,63 @@ class TimelogControllerTest < ActionController::TestCase assert_response 403 end + def test_create_with_failure + @request.session[:user_id] = 2 + post :create, :project_id => 1, + :time_entry => {:activity_id => '', + :issue_id => '', + :spent_on => '2008-03-14', + :hours => '7.3'} + + assert_response :success + assert_template 'new' + end + + def test_create_without_project + @request.session[:user_id] = 2 + assert_difference 'TimeEntry.count' do + post :create, :time_entry => {:project_id => '1', + :activity_id => '11', + :issue_id => '', + :spent_on => '2008-03-14', + :hours => '7.3'} + end + + assert_redirected_to '/projects/ecookbook/time_entries' + time_entry = TimeEntry.first(:order => 'id DESC') + assert_equal 1, time_entry.project_id + end + + def test_create_without_project_should_deny_without_permission + @request.session[:user_id] = 2 + Project.find(3).disable_module!(:time_tracking) + + assert_no_difference 'TimeEntry.count' do + post :create, :time_entry => {:project_id => '3', + :activity_id => '11', + :issue_id => '', + :spent_on => '2008-03-14', + :hours => '7.3'} + end + + assert_response 403 + end + + def test_create_without_project_with_failure + @request.session[:user_id] = 2 + assert_no_difference 'TimeEntry.count' do + post :create, :time_entry => {:project_id => '1', + :activity_id => '11', + :issue_id => '', + :spent_on => '2008-03-14', + :hours => ''} + end + + assert_response :success + assert_tag 'select', :attributes => {:name => 'time_entry[project_id]'}, + :child => {:tag => 'option', :attributes => {:value => '1', :selected => 'selected'}} + end + def test_update entry = TimeEntry.find(1) assert_equal 1, entry.issue_id @@ -289,6 +375,14 @@ class TimelogControllerTest < ActionController::TestCase :attributes => {:action => "/time_entries", :id => 'query_form'} end + def test_index_all_projects_should_show_log_time_link + @request.session[:user_id] = 2 + get :index + assert_response :success + assert_template 'index' + assert_tag 'a', :attributes => {:href => '/time_entries/new'}, :content => /Log time/ + end + def test_index_at_project_level get :index, :project_id => 'ecookbook' assert_response :success -- 2.39.5