Defect #11975

Undefined status transitions allowed in workflow (author of issue changes when selecting a new status)

Added by Brian Lindahl about 5 years ago. Updated about 5 years ago.

Status:ClosedStart date:
Priority:UrgentDue date:
Assignee:Jean-Philippe Lang% Done:

50%

Category:Issues workflow
Target version:2.1.1
Resolution:Fixed Affected version:2.1.0

Description

The modification made back in Revision 5991 allows invalid statuses to be selected by changing the author, and, thus, new_statuses_allowed_to returns values that it shouldn't. The author changes and new statuses are allowed when the form reloads when a user selects a new status.

Obviously, there still needs to be a solution for #8472, but the fix shouldn't change the author of an issue, thus breaking the transition workflow (and possibly other things).

Fix:

diff -rupN redmine-2.1.0/app/controllers/issues_controller.rb redmine-2.1.0-modified/app/controllers/issues_controller.rb
--- redmine-2.1.0/app/controllers/issues_controller.rb      2012-09-16 06:54:12.000000000 -0600
+++ redmine-2.1.0-modified/app/controllers/issues_controller.rb       2012-09-27 13:51:13.012471774 -0600
@@ -393,12 +393,12 @@ private
         end
       end
       @issue.project = @project
+      @issue.author = User.current
     else
       @issue = @project.issues.visible.find(params[:id])
     end

     @issue.project = @project
-    @issue.author = User.current
     # Tracker must be set before custom field values
     @issue.tracker ||= @project.trackers.find((params[:issue] && params[:issue][:tracker_id]) || params[:tracker_id] || :first)
     if @issue.tracker.nil?

For debugging purposes, I added this code to new_statuses_allowed_to, right before the it returns:

      status_names = statuses.collect{|s| s.name + " "}
      Rails.logger.error "**********************************************" 
      Rails.logger.error "******** author: #{author.name}" 
      Rails.logger.error "******** statuses_allowed: #{status_names}}" 
      Rails.logger.error "**********************************************" 

Before fix:

Started GET "/redmine/issues/13" for 10.79.23.119 at Thu Sep 27 13:46:34 -0600 2012
Processing by IssuesController#show as HTML
  Parameters: {"id"=>"13"}
  Current user: development.planner (id=6)
**********************************************
******** author: Reporter 1
******** statuses_allowed: New Assigned Discarded }
**********************************************
  Rendered issues/_action_menu.html.erb (14.2ms)
  Rendered issue_relations/_form.html.erb (3.8ms)
  Rendered issues/_relations.html.erb (8.4ms)
  Rendered issues/_action_menu.html.erb (4.1ms)
**********************************************
******** author: Reporter 1
******** statuses_allowed: New Assigned Discarded }
**********************************************
  Rendered issues/_form_custom_fields.html.erb (1.6ms)
  Rendered issues/_attributes.html.erb (22.2ms)
  Rendered issues/_form.html.erb (181.1ms)
  Rendered attachments/_form.html.erb (4.0ms)
  Rendered issues/_edit.html.erb (195.0ms)
  Rendered issues/_sidebar.html.erb (8.6ms)
  Rendered issues/show.html.erb within layouts/base (425.0ms)
Completed 200 OK in 2163ms (Views: 592.3ms | ActiveRecord: 37.3ms)

The form reloads when selecting the 'Assigned' status, changing the author and allowing the new 'Closed' status, which isn't defined as a valid transition for 'Development Engineer':

Started PUT "/redmine/projects/templateproduct/issues/new.js?id=13" for 10.79.23.119 at Thu Sep 27 13:46:42 -0600 2012
Processing by IssuesController#new as JS
  Parameters: {"last_journal_id"=>"", "notes"=>"", "id"=>"13", "project_id"=>"templateproduct", "attachments"=>{"1"=>{"description"=>""}}, "utf8"=>"â", "authenticity_token"=>"kg1sV89ngub8Zcxts+t5Q8H05GdmmNCFjAXjn8Pj3lo=", "issue"=>{"status_id"=>"3", "priority_id"=>"2", "fixed_version_id"=>"", "custom_field_values"=>{"5"=>"", "4"=>""}, "lock_version"=>"0", "description"=>"", "subject"=>"Reporter Feature", "category_id"=>"1", "parent_issue_id"=>""}}
  Current user: development.planner (id=6)
**********************************************
******** author: Development Engineer
******** statuses_allowed: New Assigned Discarded Closed }
**********************************************
**********************************************
******** author: Development Engineer
******** statuses_allowed: New Assigned Discarded Closed }
**********************************************
**********************************************
******** author: Development Engineer
******** statuses_allowed: New Assigned Discarded Closed }
**********************************************
**********************************************
******** author: Development Engineer
******** statuses_allowed: New Assigned Discarded Closed }
**********************************************
  Rendered issues/_form_custom_fields.html.erb (1.7ms)
  Rendered issues/_attributes.html.erb (39.9ms)
  Rendered issues/_form.html.erb (44.5ms)
  Rendered issues/_update_form.js.erb (46.6ms)
Completed 200 OK in 256ms (Views: 176.9ms | ActiveRecord: 6.4ms)

After fix:

Started GET "/redmine/issues/13" for 10.79.23.119 at Thu Sep 27 13:44:24 -0600 2012
Processing by IssuesController#show as HTML
  Parameters: {"id"=>"13"}
  Current user: development.planner (id=6)
**********************************************
******** author: Reporter 1
******** statuses_allowed: New Assigned Discarded }
**********************************************
  Rendered issues/_action_menu.html.erb (15.7ms)
  Rendered issue_relations/_form.html.erb (4.5ms)
  Rendered issues/_relations.html.erb (9.0ms)
  Rendered issues/_action_menu.html.erb (4.1ms)
**********************************************
******** author: Reporter 1
******** statuses_allowed: New Assigned Discarded }
**********************************************
  Rendered issues/_form_custom_fields.html.erb (2.1ms)
  Rendered issues/_attributes.html.erb (22.9ms)
  Rendered issues/_form.html.erb (192.9ms)
  Rendered attachments/_form.html.erb (4.3ms)
  Rendered issues/_edit.html.erb (207.0ms)
  Rendered issues/_sidebar.html.erb (8.7ms)
  Rendered issues/show.html.erb within layouts/base (455.0ms)
Completed 200 OK in 2295ms (Views: 635.5ms | ActiveRecord: 49.3ms)

The form reloads when selecting the 'Assigned' status, and does *NOT change the author and allow the 'Closed' status: *

Started PUT "/redmine/projects/templateproduct/issues/new.js?id=13" for 10.79.23.119 at Thu Sep 27 13:44:42 -0600 2012
Processing by IssuesController#new as JS
  Parameters: {"last_journal_id"=>"", "utf8"=>"â", "issue"=>{"priority_id"=>"2", "category_id"=>"1", "subject"=>"Reporter Feature", "description"=>"", "parent_issue_id"=>"", "status_id"=>"3", "fixed_version_id"=>"", "lock_version"=>"0", "custom_field_values"=>{"5"=>"", "4"=>""}}, "authenticity_token"=>"kg1sV89ngub8Zcxts+t5Q8H05GdmmNCFjAXjn8Pj3lo=", "notes"=>"", "project_id"=>"templateproduct", "attachments"=>{"1"=>{"description"=>""}}, "id"=>"13"}
  Current user: development.planner (id=6)
**********************************************
******** author: Reporter 1
******** statuses_allowed: New Assigned Discarded }
**********************************************
**********************************************
******** author: Reporter 1
******** statuses_allowed: New Assigned Discarded }
**********************************************
**********************************************
******** author: Reporter 1
******** statuses_allowed: New Assigned Discarded }
**********************************************
**********************************************
******** author: Reporter 1
******** statuses_allowed: New Assigned Discarded }
**********************************************
  Rendered issues/_form_custom_fields.html.erb (2.5ms)
  Rendered issues/_attributes.html.erb (49.6ms)
  Rendered issues/_form.html.erb (54.6ms)
  Rendered issues/_update_form.js.erb (57.6ms)
Completed 200 OK in 321ms (Views: 57.2ms | ActiveRecord: 6.7ms)

before.JPG - Before the fix is applied (before/after status is changed to 'Assigned') (15.5 KB) Brian Lindahl, 2012-09-27 22:08

after.JPG - After the fix is applied (before/after status is changed to 'Assigned') (15.9 KB) Brian Lindahl, 2012-09-27 22:08

Associated revisions

Revision 10494
Added by Jean-Philippe Lang about 5 years ago

Fixed that issue author is assigned with current user when the edit form is updated (#11975).

History

#1 Updated by Brian Lindahl about 5 years ago

Note that I wasn't able to reproduce #8472. It's likely that the root cause of #8472 was resolved in later Redmine versions (post-1.2.0), and that the fix performed in Revision 5991 is no longer necessary.

However, if the fix is still necessary, one possible solution to fix both problems could be (only speculation, since I can't reproduce #8472):

diff -rupN redmine-2.1.0/app/controllers/issues_controller.rb redmine-2.1.0-modified/app/controllers/issues_controller.rb
--- redmine-2.1.0/app/controllers/issues_controller.rb      2012-09-16 06:54:12.000000000 -0600
+++ redmine-2.1.0-modified/app/controllers/issues_controller.rb       2012-09-27 14:10:33.108460711 -0600
@@ -398,7 +398,7 @@ private
     end

     @issue.project = @project
-    @issue.author = User.current
+    @issue.author ||= User.current
     # Tracker must be set before custom field values
     @issue.tracker ||= @project.trackers.find((params[:issue] && params[:issue][:tracker_id]) || params[:tracker_id] || :first)
     if @issue.tracker.nil?

#2 Updated by Brian Lindahl about 5 years ago

Also, in my example, the only people who can perform Discarded -> Closed are the authors of the issue. Thus, the 'Development Engineer' should not be able to perform this transition. Only when the author is inappropriately changed, is this transition allowed.

#3 Updated by Jean-Philippe Lang about 5 years ago

  • Status changed from New to Resolved
  • Target version set to 2.1.1
  • Resolution set to Fixed

Fixed in r10494 with a test, thanks for pointing this out.

#4 Updated by Jean-Philippe Lang about 5 years ago

  • Status changed from Resolved to Closed

Merged into 2.1-stable.

Also available in: Atom PDF