Merged IssuesController change_status and add_note actions.

The 'Change status' specific form removed and now accessible from issue/show view with no additional request (click on 'Update' to show the form).
The 'Change issue status' permission is removed. To change the status, the user just needs to have either 'Edit' or 'Add note' permissions and some workflow transitions allowed.

git-svn-id: http://redmine.rubyforge.org/svn/trunk@1043 e93f8b46-1217-0410-a6f0-8f06a7374b81

2 files changed, 38 insertions, 44 deletions

diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb
index 39a1e2d13..11112289e 100644
--- a/app/controllers/issues_controller.rb
+++ b/app/controllers/issues_controller.rb
@@ -21,7 +21,7 @@ class IssuesController < ApplicationController
   before_filter :find_optional_project, :only => [:index, :changes]
   accept_key_auth :index, :changes
   
-  cache_sweeper :issue_sweeper, :only => [ :edit, :change_status, :destroy ]
+  cache_sweeper :issue_sweeper, :only => [ :edit, :update, :destroy ]
 
   helper :projects
   include ProjectsHelper   
@@ -82,7 +82,8 @@ class IssuesController < ApplicationController
   def show
     @custom_values = @issue.custom_values.find(:all, :include => :custom_field, :order => "#{CustomField.table_name}.position")
     @journals = @issue.journals.find(:all, :include => [:user, :details], :order => "#{Journal.table_name}.created_on ASC")
-    @status_options = @issue.status.find_new_statuses_allowed_to(User.current.role_for_project(@project), @issue.tracker)
+    @status_options = @issue.new_statuses_allowed_to(User.current)
+    @activities = Enumeration::get_values('ACTI')
     respond_to do |format|
       format.html { render :template => 'issues/show.rhtml' }
       format.pdf  { send_data(render(:template => 'issues/show.rfpdf', :layout => false), :type => 'application/pdf', :filename => "#{@project.identifier}-#{@issue.id}.pdf") }
@@ -115,47 +116,42 @@ class IssuesController < ApplicationController
     end		
   end
   
-  def add_note
+  # Attributes that can be updated on workflow transition
+  # TODO: make it configurable (at least per role)
+  UPDATABLE_ATTRS_ON_TRANSITION = %w(status_id assigned_to_id fixed_version_id done_ratio) unless const_defined?(:UPDATABLE_ATTRS_ON_TRANSITION)
+  
+  def update
+    @status_options = @issue.new_statuses_allowed_to(User.current)
+    @activities = Enumeration::get_values('ACTI')
     journal = @issue.init_journal(User.current, params[:notes])
-    attachments = attach_files(@issue, params[:attachments])
-    attachments.each {|a| journal.details << JournalDetail.new(:property => 'attachment', :prop_key => a.id, :value => a.filename)}
-    if journal.save
-      flash[:notice] = l(:notice_successful_update)
-      Mailer.deliver_issue_edit(journal) if Setting.notified_events.include?('issue_updated')
-      redirect_to :action => 'show', :id => @issue
-      return
+    # User can change issue attributes only if a workflow transition is allowed
+    if !@status_options.empty? && params[:issue]
+      attrs = params[:issue].dup
+      attrs.delete_if {|k,v| !UPDATABLE_ATTRS_ON_TRANSITION.include?(k) }
+      attrs.delete(:status_id) unless @status_options.detect {|s| s.id.to_s == attrs[:status_id].to_s}
+      @issue.attributes = attrs
     end
-    show
-  end
-
-  def change_status
-    @status_options = @issue.status.find_new_statuses_allowed_to(User.current.role_for_project(@project), @issue.tracker)
-    @new_status = IssueStatus.find(params[:new_status_id])
-    if params[:confirm]
-      begin
-        journal = @issue.init_journal(User.current, params[:notes])
-        @issue.status = @new_status
-        if @issue.update_attributes(params[:issue])
-          attachments = attach_files(@issue, params[:attachments])
-          attachments.each {|a| journal.details << JournalDetail.new(:property => 'attachment', :prop_key => a.id, :value => a.filename)}
-          # Log time
-          if current_role.allowed_to?(:log_time)
-            @time_entry ||= TimeEntry.new(:project => @project, :issue => @issue, :user => User.current, :spent_on => Date.today)
-            @time_entry.attributes = params[:time_entry]
-            @time_entry.save
-          end
-          
+    if request.post?
+      attachments = attach_files(@issue, params[:attachments])
+      attachments.each {|a| journal.details << JournalDetail.new(:property => 'attachment', :prop_key => a.id, :value => a.filename)}
+      if @issue.save
+        # Log spend time
+        if current_role.allowed_to?(:log_time)
+          @time_entry = TimeEntry.new(:project => @project, :issue => @issue, :user => User.current, :spent_on => Date.today)
+          @time_entry.attributes = params[:time_entry]
+          @time_entry.save
+        end
+        if !journal.new_record?
+          # Only send notification if something was actually changed
           flash[:notice] = l(:notice_successful_update)
           Mailer.deliver_issue_edit(journal) if Setting.notified_events.include?('issue_updated')
-          redirect_to :action => 'show', :id => @issue
         end
-      rescue ActiveRecord::StaleObjectError
-        # Optimistic locking exception
-        flash[:error] = l(:notice_locking_conflict)
+        redirect_to(params[:back_to] || {:action => 'show', :id => @issue})
       end
-    end    
-    @assignable_to = @project.members.find(:all, :include => :user).collect{ |m| m.user }
-    @activities = Enumeration::get_values('ACTI')
+    end
+  rescue ActiveRecord::StaleObjectError
+    # Optimistic locking exception
+    flash.now[:error] = l(:notice_locking_conflict)
   end
 
   def destroy
@@ -177,11 +173,11 @@ class IssuesController < ApplicationController
   def context_menu
     @priorities = Enumeration.get_values('IPRI').reverse
     @statuses = IssueStatus.find(:all, :order => 'position')
-    @allowed_statuses = @issue.status.find_new_statuses_allowed_to(User.current.role_for_project(@project), @issue.tracker)
+    @allowed_statuses = @issue.new_statuses_allowed_to(User.current)
     @assignables = @issue.assignable_users
     @assignables << @issue.assigned_to if @issue.assigned_to && !@assignables.include?(@issue.assigned_to)
     @can = {:edit => User.current.allowed_to?(:edit_issues, @project),
-            :change_status => User.current.allowed_to?(:change_issue_status, @project),
+            :assign => (@allowed_statuses.any? || User.current.allowed_to?(:edit_issues, @project)),
             :add => User.current.allowed_to?(:add_issues, @project),
             :move => User.current.allowed_to?(:move_issues, @project),
             :copy => (@project.trackers.include?(@issue.tracker) && User.current.allowed_to?(:add_issues, @project)),
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index a58eef719..2ae742dfa 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -252,11 +252,9 @@ class ProjectsController < ApplicationController
       redirect_to :controller => 'issues', :action => 'index', :project_id => @project
       return
     end
-    if current_role && User.current.allowed_to?(:change_issue_status, @project)
-      # Find potential statuses the user could be allowed to switch issues to
-      @available_statuses = Workflow.find(:all, :include => :new_status,
-                                                :conditions => {:role_id => current_role.id}).collect(&:new_status).compact.uniq
-    end
+    # Find potential statuses the user could be allowed to switch issues to
+    @available_statuses = Workflow.find(:all, :include => :new_status,
+                                              :conditions => {:role_id => current_role.id}).collect(&:new_status).compact.uniq
     render :update do |page|
       page.hide 'query_form'
       page.replace_html  'bulk-edit', :partial => 'issues/bulk_edit_form'