https://www.redmine.org/https://www.redmine.org/favicon.ico?16793021292022-05-01T13:16:16ZRedmineRedmine - Patch #37065: When someone is member of watcher group, 'watched_by' may be wrong and incompletehttps://www.redmine.org/issues/37065?journal_id=1065712022-05-01T13:16:16Zsalman mp
<ul><li><strong>File</strong> <a href="/attachments/29178">count_group_watchers_v2.patch</a> <a class="icon-only icon-download" title="Download" href="/attachments/download/29178/count_group_watchers_v2.patch">count_group_watchers_v2.patch</a> added</li></ul> Redmine - Patch #37065: When someone is member of watcher group, 'watched_by' may be wrong and incompletehttps://www.redmine.org/issues/37065?journal_id=1065762022-05-02T12:05:41ZHolger Just
<ul><li><strong>Related to</strong> <i><a class="issue tracker-2 status-5 priority-4 priority-default closed" href="/issues/4511">Feature #4511</a>: Allow adding user groups as watchers for issues</i> added</li></ul> Redmine - Patch #37065: When someone is member of watcher group, 'watched_by' may be wrong and incompletehttps://www.redmine.org/issues/37065?journal_id=1069632022-06-16T15:48:02ZHolger Just
<ul><li><strong>File</strong> <a href="/attachments/29317">0001-Respect-group-memberships-when-checking-if-an-object.patch</a> <a class="icon-only icon-download" title="Download" href="/attachments/download/29317/0001-Respect-group-memberships-when-checking-if-an-object.patch">0001-Respect-group-memberships-when-checking-if-an-object.patch</a> added</li><li><strong>Target version</strong> set to <i>Candidate for next minor release</i></li></ul><p>Attached is a patch against current trunk which fixes this behavior. The patch was extracted from <a href="https://plan.io/redmine-hosting" class="external">Planio</a>.</p>
<p>The patch is more exhaustive than the previous ones by Salman:</p>
<ul>
<li>It avoid having to load all groups as AR records</li>
<li>It also patches the instance <code>watched_by?</code> method to match the behavior of the scope.</li>
<li>It adds a test :)</li>
</ul>
<p>I have also updated the parameter name of the <code>watched_by</code> scope to make it clear that it accepts a principal (either a user or group), rather than a numeric ID. Previously, the method accepted both types. In all call-sites within Redmine however, only a user or group object is ever passed here, never a numeric ID.</p>
<p>With that being said, the way the scope is implemented, it will perform some SQL queries already when building the scope to get the groups of the passed user. This might be undesirable, e.g. if there are updates here which might affect the final query between building the scope and executing it. An alternative to the scope in the attached patch would thus be the following scope which builds a fully self-contained query where all data is only resolved during its execution:</p>
<pre><code class="ruby syntaxhl"><span class="n">scope</span> <span class="ss">:watched_by</span><span class="p">,</span> <span class="nb">lambda</span> <span class="p">{</span> <span class="o">|</span><span class="n">user_id</span><span class="o">|</span>
<span class="c1"># Basic case: return objects watched by the queried principal</span>
<span class="n">scope</span> <span class="o">=</span> <span class="n">where</span><span class="p">(</span><span class="s2">"</span><span class="si">#{</span><span class="no">Watcher</span><span class="p">.</span><span class="nf">table_name</span><span class="si">}</span><span class="s2">.user_id = ?"</span><span class="p">,</span> <span class="n">user_id</span><span class="p">)</span>
<span class="c1"># If the principal is not a group (i.e. either a User or the id of</span>
<span class="c1"># either a User or Group)...</span>
<span class="k">unless</span> <span class="n">user_id</span><span class="p">.</span><span class="nf">is_a?</span><span class="p">(</span><span class="no">Group</span><span class="p">)</span>
<span class="c1"># ... we make sure that the given id belongs to an actual User...</span>
<span class="k">if</span> <span class="n">user_id</span><span class="p">.</span><span class="nf">is_a?</span><span class="p">(</span><span class="no">Integer</span><span class="p">)</span>
<span class="n">is_user</span> <span class="o">=</span> <span class="n">where</span><span class="p">(</span><span class="no">User</span><span class="p">.</span><span class="nf">where</span><span class="p">(</span><span class="ss">id: </span><span class="n">user_id</span><span class="p">).</span><span class="nf">arel</span><span class="p">.</span><span class="nf">exists</span><span class="p">)</span>
<span class="k">else</span>
<span class="n">is_user</span> <span class="o">=</span> <span class="nb">self</span>
<span class="k">end</span>
<span class="c1"># ... and finally allow objects which are either watched by the</span>
<span class="c1"># user (base case above) or by any group the user is a member of</span>
<span class="n">scope</span> <span class="o">=</span> <span class="n">scope</span><span class="p">.</span><span class="nf">or</span><span class="p">(</span>
<span class="n">is_user</span><span class="p">.</span>
<span class="nf">where</span><span class="p">(</span><span class="s2">"</span><span class="si">#{</span><span class="no">Watcher</span><span class="p">.</span><span class="nf">table_name</span><span class="si">}</span><span class="s2">.user_id IN (?)"</span><span class="p">,</span> <span class="no">Group</span><span class="p">.</span><span class="nf">having_user</span><span class="p">(</span><span class="n">user_id</span><span class="p">).</span><span class="nf">select</span><span class="p">(</span><span class="ss">:id</span><span class="p">))</span>
<span class="p">)</span>
<span class="k">end</span>
<span class="c1"># Finally, we have to join the watchers table at the end to simplify</span>
<span class="c1"># the or condition above. The final SQL is the same in any case.</span>
<span class="n">scope</span><span class="p">.</span><span class="nf">joins</span><span class="p">(</span><span class="ss">:watchers</span><span class="p">)</span>
<span class="p">}</span>
</code></pre>
<p>This scope is functionally equivalent to the version in the patch (with the addition that we do support passed Integer arguments for the id of a user or group here). It will not make any queries on its own, thus avoiding additional roundtrips to the database. With that however, it will also not use any cached data (such as a previously cached <code>principal.group_ids</code> list).</p> Redmine - Patch #37065: When someone is member of watcher group, 'watched_by' may be wrong and incompletehttps://www.redmine.org/issues/37065?journal_id=1069972022-06-19T20:04:41ZMarius BĂLTEANU
<ul><li><strong>Category</strong> set to <i>Email notifications</i></li><li><strong>Assignee</strong> set to <i>Marius BĂLTEANU</i></li><li><strong>Target version</strong> changed from <i>Candidate for next minor release</i> to <i>5.0.2</i></li></ul><p>I think it is safe to deliver this fix in <a class="version" href="https://www.redmine.org/versions/180">5.0.2</a>.</p> Redmine - Patch #37065: When someone is member of watcher group, 'watched_by' may be wrong and incompletehttps://www.redmine.org/issues/37065?journal_id=1070052022-06-20T05:53:57ZMarius BĂLTEANU
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>Resolved</i></li></ul><p>Patch committed, thanks!</p> Redmine - Patch #37065: When someone is member of watcher group, 'watched_by' may be wrong and incompletehttps://www.redmine.org/issues/37065?journal_id=1070112022-06-20T15:20:32ZMarius BĂLTEANU
<ul><li><strong>Status</strong> changed from <i>Resolved</i> to <i>Closed</i></li></ul> Redmine - Patch #37065: When someone is member of watcher group, 'watched_by' may be wrong and incompletehttps://www.redmine.org/issues/37065?journal_id=1070622022-06-23T15:01:58Zsalman mp
<ul></ul><p>Holger Just wrote:</p>
<blockquote>
<p>The patch is more exhaustive than the previous ones by Salman:</p>
</blockquote>
<p>Good job. Thanks.</p>