https://www.redmine.org/https://www.redmine.org/favicon.ico?16793021292022-09-05T14:21:15ZRedmineRedmine - Defect #37635: Respect Role#consider_workflow? when checking for allowed status transitionshttps://www.redmine.org/issues/37635?journal_id=1078072022-09-05T14:21:15ZGo MAEDA
<ul><li><strong>Tracker</strong> changed from <i>Patch</i> to <i>Defect</i></li><li><strong>Status</strong> changed from <i>New</i> to <i>Confirmed</i></li><li><strong>Target version</strong> set to <i>5.1.0</i></li></ul><p>Setting the target version to 5.1.0.</p> Redmine - Defect #37635: Respect Role#consider_workflow? when checking for allowed status transitionshttps://www.redmine.org/issues/37635?journal_id=1078092022-09-06T12:57:30ZMischa The Evil
<ul><li><strong>Related to</strong> <i><a class="issue tracker-1 status-5 priority-4 priority-default closed" href="/issues/37229">Defect #37229</a>: Ignore workflow transitions that already defined and now is not appliable</i> added</li></ul> Redmine - Defect #37635: Respect Role#consider_workflow? when checking for allowed status transitionshttps://www.redmine.org/issues/37635?journal_id=1078112022-09-06T15:23:51ZMischa The Evil
<ul></ul><p>Just a thought:</p>
<p>The following code in <code>Issue#workflow_rule_by_attribute</code> (<a class="source" href="https://www.redmine.org/projects/redmine/repository/svn/revisions/21799/entry/trunk/app/models/issue.rb#L680">source:/trunk/app/models/issue.rb@21799#L680</a>) exists:<br /><pre><code class="ruby syntaxhl"> <span class="n">user_real</span> <span class="o">=</span> <span class="n">user</span> <span class="o">||</span> <span class="no">User</span><span class="p">.</span><span class="nf">current</span>
<span class="n">roles</span> <span class="o">=</span> <span class="n">user_real</span><span class="p">.</span><span class="nf">admin</span> <span class="p">?</span> <span class="no">Role</span><span class="p">.</span><span class="nf">all</span><span class="p">.</span><span class="nf">to_a</span> <span class="p">:</span> <span class="n">user_real</span><span class="p">.</span><span class="nf">roles_for_project</span><span class="p">(</span><span class="n">project</span><span class="p">)</span>
<span class="n">roles</span> <span class="o">=</span> <span class="n">roles</span><span class="p">.</span><span class="nf">select</span><span class="p">(</span><span class="o">&</span><span class="ss">:consider_workflow?</span><span class="p">)</span>
</code></pre></p>
<p>The following code addition is proposed in this patch:<br /><pre><code class="ruby syntaxhl"> <span class="n">roles</span> <span class="o">=</span> <span class="n">user</span><span class="p">.</span><span class="nf">admin</span> <span class="p">?</span> <span class="no">Role</span><span class="p">.</span><span class="nf">all</span><span class="p">.</span><span class="nf">to_a</span> <span class="p">:</span> <span class="n">user</span><span class="p">.</span><span class="nf">roles_for_project</span><span class="p">(</span><span class="n">project</span><span class="p">)</span>
<span class="n">roles</span> <span class="o">=</span> <span class="n">roles</span><span class="p">.</span><span class="nf">select</span><span class="p">(</span><span class="o">&</span><span class="ss">:consider_workflow?</span><span class="p">)</span>
</code></pre></p>
<p>Wouldn't it be better to extract this code into a new (helper) method given the similarity?</p> Redmine - Defect #37635: Respect Role#consider_workflow? when checking for allowed status transitionshttps://www.redmine.org/issues/37635?journal_id=1079132022-09-15T13:32:43ZGo MAEDA
<ul></ul><p>Mischa The Evil wrote:</p>
<blockquote>
<p>Wouldn't it be better to extract this code into a new (helper) method given the similarity?</p>
</blockquote>
<p>What do you think about extracting the code as follows?</p>
<pre><code class="diff syntaxhl"><span class="gh">diff --git a/app/models/issue.rb b/app/models/issue.rb
index 84907a475..a5bd372d5 100644
</span><span class="gd">--- a/app/models/issue.rb
</span><span class="gi">+++ b/app/models/issue.rb
</span><span class="p">@@ -2053,4 +2053,9 @@</span> class Issue < ActiveRecord::Base
Project
end
end
<span class="gi">+
+ def roles_for_workflow(user = User.current)
+ roles = user.admin ? Role.all.to_a : user.roles_for_project(project)
+ roles.select(&:consider_workflow?)
+ end
</span> end
</code></pre> Redmine - Defect #37635: Respect Role#consider_workflow? when checking for allowed status transitionshttps://www.redmine.org/issues/37635?journal_id=1079142022-09-15T13:44:57ZHolger Just
<ul><li><strong>File</strong> <a href="/attachments/29680">0001-Consider-only-roles-with-either-add_issues-or-edit_i.patch</a> <a class="icon-only icon-download" title="Download" href="/attachments/download/29680/0001-Consider-only-roles-with-either-add_issues-or-edit_i.patch">0001-Consider-only-roles-with-either-add_issues-or-edit_i.patch</a> added</li></ul><p>I'd require the user argument (rather than allowing a default). Then, this looks good for me.</p>
<p>I have attached an updated patch which uses this new extracted method.</p> Redmine - Defect #37635: Respect Role#consider_workflow? when checking for allowed status transitionshttps://www.redmine.org/issues/37635?journal_id=1079272022-09-17T06:14:40ZGo MAEDA
<ul><li><strong>Status</strong> changed from <i>Confirmed</i> to <i>Closed</i></li><li><strong>Assignee</strong> set to <i>Go MAEDA</i></li><li><strong>Resolution</strong> set to <i>Fixed</i></li></ul><p>Committed the fix. Thank you.</p> Redmine - Defect #37635: Respect Role#consider_workflow? when checking for allowed status transitionshttps://www.redmine.org/issues/37635?journal_id=1089912023-01-04T05:04:07ZGo MAEDA
<ul><li><strong>Has duplicate</strong> <i><a class="issue tracker-1 status-5 priority-4 priority-default closed" href="/issues/28078">Defect #28078</a>: Workflows inconsistencies when removing "add/edit issue" permission to a role which already has a workflow defined</i> added</li></ul> Redmine - Defect #37635: Respect Role#consider_workflow? when checking for allowed status transitionshttps://www.redmine.org/issues/37635?journal_id=1113732023-11-01T22:17:50ZMischa The Evil
<ul><li><strong>Related to</strong> <i><a class="issue tracker-1 status-9 priority-4 priority-default" href="/issues/39493">Defect #39493</a>: Role with only :edit_own_issues no longer considered for workflow</i> added</li></ul>