Defect #37635

Respect Role#consider_workflow? when checking for allowed status transitions

Added by Holger Just about 1 month ago. Updated 16 days ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Go MAEDA% Done:

0%

Category:Issues workflow
Target version:5.1.0
Resolution:Fixed Affected version:

Description

When defining workflow rules for a role and tracker, we generally check if a role can have any workflows at all. Right now, only roles which have at least one of the edit_issues or add_issue permissions can have any workflows. We check this when displaying editable workflows for admins.

Now, it is possible that the database holds workflow rules for roles which currently do not have one of those permissions (but might have had in the past). Currently, when checking for the allowed issue statuses in Issue#new_statuses_allowed_to (which is used to validate possible new issue statuses for an issue), we do not take into account if the defined workflow rules shall be considered at all.

This might result in workflow rules take into account which can not be (directly) edited by admins, resulting in a rather surprising behavior.

The attached patch changes this by ensuring that we only use workflow transitions which are defined for roles having one of the permissions. Workflow transitions for other roles are ignored.

0001-Consider-only-roles-with-either-add_issues-or-edit_i.patch Magnifier (2.78 KB) Holger Just, 2022-09-01 20:49

0001-Consider-only-roles-with-either-add_issues-or-edit_i.patch Magnifier - Updated patch from #37635#note-5 (3.28 KB) Holger Just, 2022-09-15 15:44


Related issues

Related to Redmine - Defect #37229: Ignore workflow transitions that already defined and now ... Closed

Associated revisions

Revision 21817
Added by Go MAEDA 16 days ago

Consider only roles with either add_issues or edit_issues permissions for any status transitions (#37635).

Patch by Holger Just.

History

#1 Updated by Go MAEDA 28 days ago

  • Tracker changed from Patch to Defect
  • Status changed from New to Confirmed
  • Target version set to 5.1.0

Setting the target version to 5.1.0.

#2 Updated by Mischa The Evil 27 days ago

  • Related to Defect #37229: Ignore workflow transitions that already defined and now is not appliable added

#3 Updated by Mischa The Evil 27 days ago

Just a thought:

The following code in Issue#workflow_rule_by_attribute (source:/trunk/app/models/issue.rb@21799#L680) exists:

    user_real = user || User.current
    roles = user_real.admin ? Role.all.to_a : user_real.roles_for_project(project)
    roles = roles.select(&:consider_workflow?)

The following code addition is proposed in this patch:

    roles = user.admin ? Role.all.to_a : user.roles_for_project(project)
    roles = roles.select(&:consider_workflow?)

Wouldn't it be better to extract this code into a new (helper) method given the similarity?

#4 Updated by Go MAEDA 18 days ago

Mischa The Evil wrote:

Wouldn't it be better to extract this code into a new (helper) method given the similarity?

What do you think about extracting the code as follows?

diff --git a/app/models/issue.rb b/app/models/issue.rb
index 84907a475..a5bd372d5 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -2053,4 +2053,9 @@ class Issue < ActiveRecord::Base
       Project
     end
   end
+
+  def roles_for_workflow(user = User.current)
+    roles = user.admin ? Role.all.to_a : user.roles_for_project(project)
+    roles.select(&:consider_workflow?)
+  end
 end

#5 Updated by Holger Just 18 days ago

I'd require the user argument (rather than allowing a default). Then, this looks good for me.

I have attached an updated patch which uses this new extracted method.

#6 Updated by Go MAEDA 16 days ago

  • Status changed from Confirmed to Closed
  • Assignee set to Go MAEDA
  • Resolution set to Fixed

Committed the fix. Thank you.

Also available in: Atom PDF