Merged r7920, r7921, r7922 and r7924 from trunk (#9405).

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/branches/1.2-stable@8158 e93f8b46-1217-0410-a6f0-8f06a7374b81

7 files changed, 67 insertions, 10 deletions

diff --git a/app/controllers/context_menus_controller.rb b/app/controllers/context_menus_controller.rb
index 4d69f4f04..1ecd73851 100644
--- a/app/controllers/context_menus_controller.rb
+++ b/app/controllers/context_menus_controller.rb
@@ -48,9 +48,8 @@ class ContextMenusController < ApplicationController
     @projects = @time_entries.collect(&:project).compact.uniq
     @project = @projects.first if @projects.size == 1
     @activities = TimeEntryActivity.shared.active
-    @can = {:edit   => User.current.allowed_to?(:log_time, @projects),
-            :update => User.current.allowed_to?(:log_time, @projects),
-            :delete => User.current.allowed_to?(:log_time, @projects)
+    @can = {:edit   => User.current.allowed_to?(:edit_time_entries, @projects),
+            :delete => User.current.allowed_to?(:edit_time_entries, @projects)
             }
     @back = back_url
     render :layout => false
diff --git a/app/views/context_menus/time_entries.html.erb b/app/views/context_menus/time_entries.html.erb
index ed6ba4ee7..14708e3f0 100644
--- a/app/views/context_menus/time_entries.html.erb
+++ b/app/views/context_menus/time_entries.html.erb
@@ -15,13 +15,13 @@
 		<ul>
 		<% @activities.each do |u| -%>
 		    <li><%= context_menu_link u.name, {:controller => 'timelog', :action => 'bulk_edit', :ids => @time_entries.collect(&:id), :time_entry => {'activity_id' => u}, :back_url => @back}, :method => :post,
-		                              :selected => (@time_entry && u == @time_entry.activity), :disabled => !@can[:update] %></li>
+		                              :selected => (@time_entry && u == @time_entry.activity), :disabled => !@can[:edit] %></li>
 		<% end -%>
 		    <li><%= context_menu_link l(:label_nobody), {:controller => 'timelog', :action => 'bulk_edit', :ids => @time_entries.collect(&:id), :time_entry => {'activity_id' => 'none'}, :back_url => @back}, :method => :post,
-		                              :selected => (@time_entry && @time_entry.activity.nil?), :disabled => !@can[:update] %></li>
+		                              :selected => (@time_entry && @time_entry.activity.nil?), :disabled => !@can[:edit] %></li>
 		</ul>
 	</li>
-	<% end %>
+	<% end %>

 
   <%= call_hook(:view_time_entries_context_menu_end, {:time_entries => @time_entries, :can => @can, :back => @back }) %>
 
diff --git a/app/views/issues/_edit.rhtml b/app/views/issues/_edit.rhtml
index ec36b1459..e43e9df3a 100644
--- a/app/views/issues/_edit.rhtml
+++ b/app/views/issues/_edit.rhtml
@@ -15,7 +15,7 @@
         <%= render :partial => (@edit_allowed ? 'form' : 'form_update'), :locals => {:f => f} %>
         </fieldset>
     <% end %>
-    <% if authorize_for('timelog', 'edit') %>
+    <% if User.current.allowed_to?(:log_time, @project) %>
         <fieldset class="tabular"><legend><%= l(:button_log_time) %></legend>
         <% fields_for :time_entry, @time_entry, { :builder => TabularFormBuilder, :lang => current_language} do |time_entry| %>
         <div class="splitcontentleft">
diff --git a/lib/redmine.rb b/lib/redmine.rb
index 283f0250f..35058b3f4 100644
--- a/lib/redmine.rb
+++ b/lib/redmine.rb
@@ -88,10 +88,10 @@ Redmine::AccessControl.map do |map|
   end
   
   map.project_module :time_tracking do |map|
-    map.permission :log_time, {:timelog => [:new, :create, :edit, :update, :bulk_edit, :bulk_update]}, :require => :loggedin
+    map.permission :log_time, {:timelog => [:new, :create]}, :require => :loggedin
     map.permission :view_time_entries, :timelog => [:index, :show], :time_entry_reports => [:report]
-    map.permission :edit_time_entries, {:timelog => [:new, :create, :edit, :update, :destroy, :bulk_edit, :bulk_update]}, :require => :member
-    map.permission :edit_own_time_entries, {:timelog => [:new, :create, :edit, :update, :destroy,:bulk_edit, :bulk_update]}, :require => :loggedin
+    map.permission :edit_time_entries, {:timelog => [:edit, :update, :destroy, :bulk_edit, :bulk_update]}, :require => :member
+    map.permission :edit_own_time_entries, {:timelog => [:edit, :update, :destroy,:bulk_edit, :bulk_update]}, :require => :loggedin
     map.permission :manage_project_activities, {:project_enumerations => [:update, :destroy]}, :require => :member
   end
   
diff --git a/test/functional/context_menus_controller_test.rb b/test/functional/context_menus_controller_test.rb
index 67e919a9c..38eabe4ac 100644
--- a/test/functional/context_menus_controller_test.rb
+++ b/test/functional/context_menus_controller_test.rb
@@ -115,4 +115,23 @@ class ContextMenusControllerTest < ActionController::TestCase
     assert_template 'context_menu'
     assert_equal [1], assigns(:issues).collect(&:id)
   end
+  
+  def test_time_entries_context_menu
+    @request.session[:user_id] = 2
+    get :time_entries, :ids => [1, 2]
+    assert_response :success
+    assert_template 'time_entries'
+    assert_tag 'a', :content => 'Edit'
+    assert_no_tag 'a', :content => 'Edit', :attributes => {:class => /disabled/}
+  end
+  
+  def test_time_entries_context_menu_without_edit_permission
+    @request.session[:user_id] = 2
+    Role.find_by_name('Manager').remove_permission! :edit_time_entries
+    
+    get :time_entries, :ids => [1, 2]
+    assert_response :success
+    assert_template 'time_entries'
+    assert_tag 'a', :content => 'Edit', :attributes => {:class => /disabled/}
+  end
 end
diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb
index f3960fa79..2c824e903 100644
--- a/test/functional/issues_controller_test.rb
+++ b/test/functional/issues_controller_test.rb
@@ -872,6 +872,22 @@ class IssuesControllerTest < ActionController::TestCase
     assert_equal Issue.find(1), assigns(:issue)
   end
 
+  def test_get_edit_should_display_the_time_entry_form_with_log_time_permission
+    @request.session[:user_id] = 2
+    Role.find_by_name('Manager').update_attribute :permissions, [:view_issues, :edit_issues, :log_time]
+    
+    get :edit, :id => 1
+    assert_tag 'input', :attributes => {:name => 'time_entry[hours]'}
+  end
+
+  def test_get_edit_should_not_display_the_time_entry_form_without_log_time_permission
+    @request.session[:user_id] = 2
+    Role.find_by_name('Manager').remove_permission! :log_time
+    
+    get :edit, :id => 1
+    assert_no_tag 'input', :attributes => {:name => 'time_entry[hours]'}
+  end
+
   def test_get_edit_with_params
     @request.session[:user_id] = 2
     get :edit, :id => 1, :issue => { :status_id => 5, :priority_id => 7 },
diff --git a/test/functional/timelog_controller_test.rb b/test/functional/timelog_controller_test.rb
index 611420a28..b5aae3680 100644
--- a/test/functional/timelog_controller_test.rb
+++ b/test/functional/timelog_controller_test.rb
@@ -115,6 +115,18 @@ class TimelogControllerTest < ActionController::TestCase
     assert_equal 3, t.user_id
   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
+    post :create, :project_id => 1,
+                :time_entry => {:activity_id => '11',
+                                :issue_id => '',
+                                :spent_on => '2008-03-14',
+                                :hours => '7.3'}
+
+    assert_response 403
+  end
+
   def test_update
     entry = TimeEntry.find(1)
     assert_equal 1, entry.issue_id
@@ -161,6 +173,9 @@ class TimelogControllerTest < ActionController::TestCase
 
   def test_bulk_update_on_different_projects
     @request.session[:user_id] = 2
+    # makes user a manager on the other project
+    Member.create!(:user_id => 2, :project_id => 3, :role_ids => [1])
+    
     # update time entry activity
     post :bulk_update, :ids => [1, 2, 4], :time_entry => { :activity_id => 9 }
     
@@ -203,6 +218,14 @@ class TimelogControllerTest < ActionController::TestCase
     assert_redirected_to :controller => 'timelog', :action => 'index', :project_id => Project.find(1).identifier
   end
   
+  def test_post_bulk_update_without_edit_permission_should_be_denied
+    @request.session[:user_id] = 2
+    Role.find_by_name('Manager').remove_permission! :edit_time_entries
+    post :bulk_update, :ids => [1,2]
+
+    assert_response 403
+  end
+
   def test_destroy
     @request.session[:user_id] = 2
     delete :destroy, :id => 1