From 976a31e941e61542075866563e4c0740106c5d70 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sun, 6 Jan 2008 17:06:14 +0000 Subject: [PATCH] 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 --- app/controllers/issues_controller.rb | 74 ++++++------ app/controllers/projects_controller.rb | 8 +- app/models/issue.rb | 7 ++ app/models/issue_status.rb | 4 + app/views/issues/_bulk_edit_form.rhtml | 2 +- app/views/issues/_update.rhtml | 42 +++++++ app/views/issues/change_status.rhtml | 38 ------ app/views/issues/context_menu.rhtml | 12 +- app/views/issues/show.rhtml | 30 ++--- app/views/issues/update.rhtml | 4 + lang/bg.yml | 2 + lang/cs.yml | 2 + lang/de.yml | 2 + lang/en.yml | 2 + lang/es.yml | 2 + lang/fr.yml | 2 + lang/he.yml | 2 + lang/it.yml | 2 + lang/ja.yml | 2 + lang/ko.yml | 2 + lang/nl.yml | 2 + lang/pl.yml | 2 + lang/pt-br.yml | 2 + lang/pt.yml | 2 + lang/ro.yml | 2 + lang/ru.yml | 2 + lang/sr.yml | 2 + lang/sv.yml | 2 + lang/zh-tw.yml | 2 + lang/zh.yml | 2 + lib/redmine.rb | 5 +- lib/redmine/default_data/loader.rb | 3 - test/fixtures/enumerations.yml | 9 ++ test/fixtures/roles.yml | 5 +- test/fixtures/users.yml | 23 ++++ test/functional/issues_controller_test.rb | 122 ++++++++++++++++++-- test/integration/issues_test.rb | 4 +- test/test_helper.rb | 6 +- vendor/plugins/gloc-1.1.0/lib/gloc-rails.rb | 21 ++++ 39 files changed, 326 insertions(+), 133 deletions(-) create mode 100644 app/views/issues/_update.rhtml delete mode 100644 app/views/issues/change_status.rhtml create mode 100644 app/views/issues/update.rhtml 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' diff --git a/app/models/issue.rb b/app/models/issue.rb index f7b01ea6a..419c6cdc7 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -180,6 +180,13 @@ class Issue < ActiveRecord::Base project.assignable_users end + # Returns an array of status that user is able to apply + def new_statuses_allowed_to(user) + statuses = status.find_new_statuses_allowed_to(user.role_for_project(project), tracker) + statuses << status unless statuses.empty? + statuses.uniq.sort + end + # Returns the mail adresses of users that should be notified for the issue def recipients recipients = project.recipients diff --git a/app/models/issue_status.rb b/app/models/issue_status.rb index a5d228405..ddff9c005 100644 --- a/app/models/issue_status.rb +++ b/app/models/issue_status.rb @@ -56,6 +56,10 @@ class IssueStatus < ActiveRecord::Base false end + def <=>(status) + position <=> status.position + end + def to_s; name end private diff --git a/app/views/issues/_bulk_edit_form.rhtml b/app/views/issues/_bulk_edit_form.rhtml index bc3f62e6d..e9e1cef86 100644 --- a/app/views/issues/_bulk_edit_form.rhtml +++ b/app/views/issues/_bulk_edit_form.rhtml @@ -2,7 +2,7 @@
<%= l(:label_bulk_edit_selected_issues) %>

-<% if @available_statuses %> +<% if @available_statuses.any? %> <% end %> diff --git a/app/views/issues/_update.rhtml b/app/views/issues/_update.rhtml new file mode 100644 index 000000000..3cf593806 --- /dev/null +++ b/app/views/issues/_update.rhtml @@ -0,0 +1,42 @@ +<% labelled_tabular_form_for(:issue, @issue, :url => {:action => 'update', :id => @issue}, :html => {:multipart => true}) do |f| %> + +

+<% unless @status_options.empty? %> +<%= f.hidden_field :lock_version %> +
<%= l(:label_change_properties) %> +
+

<%= f.select :status_id, (@status_options.collect {|p| [p.name, p.id]}), :required => true %>

+

<%= f.select :assigned_to_id, (@issue.assignable_users.collect {|m| [m.name, m.id]}), :include_blank => true %>

+
+
+

<%= f.select :done_ratio, ((0..10).to_a.collect {|r| ["#{r*10} %", r*10] }) %>

+

<%= f.select :fixed_version_id, (@project.versions.sort.collect {|v| [v.name, v.id]}), { :include_blank => true } %>

+
+
+<% end%> +<% if authorize_for('timelog', 'edit') %> +
<%= l(:button_log_time) %> + <% fields_for :time_entry, @time_entry, { :builder => TabularFormBuilder, :lang => current_language} do |time_entry| %> +
+

<%= time_entry.text_field :hours, :size => 6, :label => :label_spent_time %> <%= l(:field_hours) %>

+
+
+

<%= time_entry.text_field :comments, :size => 40 %>

+

<%= time_entry.select :activity_id, (@activities.collect {|p| [p.name, p.id]}) %>

+
+ <% end %> +
+<% end %> + +
<%= l(:field_notes) %> +<%= text_area_tag 'notes', @notes, :cols => 60, :rows => 10, :class => 'wiki-edit' %> +<%= wikitoolbar_for 'notes' %> + +

+<%= file_field_tag 'attachments[]', :size => 30 %> (<%= l(:label_max_size) %>: <%= number_to_human_size(Setting.attachment_max_size.to_i.kilobytes) %>)

+
+
+ +<%= submit_tag l(:button_submit) %> +<% end %> diff --git a/app/views/issues/change_status.rhtml b/app/views/issues/change_status.rhtml deleted file mode 100644 index a1e294556..000000000 --- a/app/views/issues/change_status.rhtml +++ /dev/null @@ -1,38 +0,0 @@ -

<%=l(:label_issue)%> #<%= @issue.id %>: <%=h @issue.subject %>

- -<%= error_messages_for 'issue' %> -<% labelled_tabular_form_for(:issue, @issue, :url => {:action => 'change_status', :id => @issue}, :html => {:multipart => true}) do |f| %> - -<%= hidden_field_tag 'confirm', 1 %> -<%= hidden_field_tag 'new_status_id', @new_status.id %> -<%= f.hidden_field :lock_version %> - -
-
-

<%= @new_status.name %>

-

<%= f.select :assigned_to_id, (@issue.assignable_users.collect {|m| [m.name, m.id]}), :include_blank => true %>

-

<%= f.select :done_ratio, ((0..10).to_a.collect {|r| ["#{r*10} %", r*10] }) %>

-

<%= f.select :fixed_version_id, (@project.versions.sort.collect {|v| [v.name, v.id]}), { :include_blank => true } %>

-
-
-<% if authorize_for('timelog', 'edit') %> -<% fields_for :time_entry, @time_entry, { :builder => TabularFormBuilder, :lang => current_language} do |time_entry| %> -

<%= time_entry.text_field :hours, :size => 6, :label => :label_spent_time %> <%= l(:field_hours) %>

-

<%= time_entry.text_field :comments, :size => 40 %>

-

<%= time_entry.select :activity_id, (@activities.collect {|p| [p.name, p.id]}) %>

-<% end %> -<% end %> -
- -
- -

-<%= text_area_tag 'notes', @notes, :cols => 60, :rows => 10, :class => 'wiki-edit' %>

- -

-<%= file_field_tag 'attachments[]', :size => 30 %> (<%= l(:label_max_size) %>: <%= number_to_human_size(Setting.attachment_max_size.to_i.kilobytes) %>)

-
- -<%= submit_tag l(:button_save) %> -<% end %> diff --git a/app/views/issues/context_menu.rhtml b/app/views/issues/context_menu.rhtml index 3af49fb04..0f11bb943 100644 --- a/app/views/issues/context_menu.rhtml +++ b/app/views/issues/context_menu.rhtml @@ -6,8 +6,8 @@ <%= l(:field_status) %> @@ -24,11 +24,11 @@ <%= l(:field_assigned_to) %>
  • <%= context_menu_link l(:button_copy), {:controller => 'projects', :action => 'add_issue', :id => @project, :copy_from => @issue}, diff --git a/app/views/issues/show.rhtml b/app/views/issues/show.rhtml index 91b638216..8cd44424c 100644 --- a/app/views/issues/show.rhtml +++ b/app/views/issues/show.rhtml @@ -1,5 +1,5 @@
    -<%= show_and_goto_link(l(:label_add_note), 'add-note', :class => 'icon icon-note') if authorize_for('issues', 'add_note') %> +<%= show_and_goto_link(l(:button_update), 'update', :class => 'icon icon-note') if authorize_for('issues', 'update') %> <%= link_to_if_authorized l(:button_edit), {:controller => 'issues', :action => 'edit', :id => @issue}, :class => 'icon icon-edit', :accesskey => accesskey(:edit) %> <%= link_to_if_authorized l(:button_log_time), {:controller => 'timelog', :action => 'edit', :issue_id => @issue}, :class => 'icon icon-time' %> <%= watcher_tag(@issue, User.current) %> @@ -81,16 +81,6 @@ end %>
    -<% if authorize_for('issues', 'change_status') and @status_options and !@status_options.empty? %> - <% form_tag({:controller => 'issues', :action => 'change_status', :id => @issue}) do %> -

    <%=l(:label_change_status)%> : - - <%= submit_tag l(:button_change) %>

    - <% end %> -<% end %> - <% if @journals.any? %>

    <%=l(:label_history)%>

    @@ -98,18 +88,12 @@ end %>
    <% end %> -<% if authorize_for('issues', 'add_note') %> - -