Feature #4511

Allow adding user groups as watchers for issues

Added by Michael Ruder over 10 years ago. Updated about 1 month ago.

Status:ClosedStart date:2010-01-01
Priority:NormalDue date:
Assignee:Go MAEDA% Done:

0%

Category:Issues
Target version:4.2.0
Resolution:Fixed

Description

Having the nice user group feature in Redmine 0.9, it would be very handy to be able to add whole group as observers for issues. Currently, only user accounts can be added as observers.

Issue page.JPG (22.6 KB) cleber souza, 2018-04-25 15:18

4511-add-group-to-watcher.patch Magnifier (24.2 KB) Yuichi HARADA, 2020-01-31 03:18

4511-add-group-to-watcher-v2.patch Magnifier - Fixed RuboCop offences (24.2 KB) Go MAEDA, 2020-01-31 07:55

Screenshot_add-watcher.png (42.5 KB) Go MAEDA, 2020-01-31 08:03

Screenshot_watchers-list.png (13.9 KB) Go MAEDA, 2020-01-31 08:03

4511-add-group-to-watcher-v3.patch Magnifier (24.6 KB) Yuichi HARADA, 2020-02-06 06:23

0003-Use-principals-in-acts_as_watchable.patch Magnifier (1.09 KB) Marius BALTEANU, 2020-04-25 09:31

0004-Use-scope-assignable_watchers.patch Magnifier (4.25 KB) Marius BALTEANU, 2020-04-25 09:31

0002-Use-Principal-to-get-users-and-groups-for-watchers-i.patch Magnifier (2.36 KB) Marius BALTEANU, 2020-04-25 09:31

0001-Get-the-list-of-new-issue-watchers-using-single-quer.patch Magnifier (1.23 KB) Marius BALTEANU, 2020-04-25 09:31


Related issues

Related to Redmine - Feature #10121: Watchers - Add Group / Role New
Related to Redmine - Feature #13513: Personal watch note on issue New
Related to Redmine - Feature #4244: Multiple email addresses for each user Closed 2009-11-19
Related to Redmine - Feature #29501: Allow addition of watcher group via bulk edit context menu Closed
Related to Redmine - Patch #33036: Add missing fixture to IssuesControllerTest Closed
Related to Redmine - Patch #33100: Fix a test - Issue watchers are not always sorted by id Closed
Duplicated by Redmine - Feature #24943: Issue watchers to have groups in addition to users Closed
Duplicated by Redmine - Feature #15164: Make it possible to add group as a watcher Closed

Associated revisions

Revision 19498
Added by Go MAEDA 5 months ago

Allow adding user groups as watchers for issues (#4511).

Patch by Yuichi HARADA.

Revision 19507
Added by Go MAEDA 5 months ago

Add a missing fixture for IssuesControllerTest#test_post_create_with_watchers (#4511).

Revision 19508
Added by Go MAEDA 5 months ago

Reverts r19507 (#4511).

Revision 19526
Added by Go MAEDA 5 months ago

Add a missing fixture to IssuesControllerTest (#4511, #33036).

Patch by Yuichi HARADA.

Revision 19562
Added by Go MAEDA 4 months ago

Fix that IssuesControllerTest occasionally fails (#33100, #4511).

Patch by Vincent Robert.

Revision 19723
Added by Go MAEDA 3 months ago

Get the list of new issue watchers using single query and limit the results to 20 (#4511).

Patch by Marius BALTEANU.

Revision 19724
Added by Go MAEDA 3 months ago

Use Principal to get users and groups for watchers in watchers controller (#4511).

Patch by Marius BALTEANU.

Revision 19725
Added by Go MAEDA 3 months ago

Use principals in acts_as_watchable (#4511).

Patch by Marius BALTEANU.

Revision 19726
Added by Go MAEDA 3 months ago

Use scope assignable_watchers (#4511).

Patch by Marius BALTEANU.

History

#1 Updated by Felix Schäfer over 10 years ago

There's already a FR for this at #2964, as well as some discussion regarding this.

#2 Updated by Michael Ruder over 10 years ago

Not exactly, #2964 is about assigning the issue to multiple users. I read the discussion about it. Right now, there can be only one asignee of an issue and having suddenly several raises indeed some questions.

With observers, there is already the option to add multiple to one issue. My request is about being able not only to add users as observers but entire groups. We often have support issues which several employees of our customer want to follow. Adding them one by one to each issue is a bit of a hassle. I would like to be able to just create a group and add this group as observer.

#3 Updated by Felix Schäfer over 10 years ago

Michael Ruder wrote:

Not exactly, #2964 is about assigning the issue to multiple users.

Oh, sorry, I misread that you wanted to assign an issue to a group, not make a group observe an issue. My bad.

#4 Updated by Eric Davis over 10 years ago

  • Subject changed from Allowing to add user groups as observers for issues to Allowing to add user groups as watchers for issues
  • Category changed from Groups to Issues

+1 I would expect they would be added and removed as a group as opposed to adding them as a group and having to remove each member.

Example:

  • Add GroupA - (Developer1 and Developer2)
  • Remove GroupA

#5 Updated by Pavel Smirnov over 10 years ago

this is is think is a really good idea, will save a lot of time

#6 Updated by Alain V. over 10 years ago

I think also this a good idea, will help us a lot!
I vote for this! +1

#7 Updated by Serge Kosse over 10 years ago

I join and vote too.
Very necessary functional, IMHO

#8 Updated by Enrique Delgado almost 10 years ago

+1

#11 Updated by Novikov Igor almost 10 years ago

Would be very functional, will really reduce timecost for big projects... +1

#12 Updated by Milan Stastny almost 10 years ago

got this request as issue in our company. Gonna try to make it as a plugin, when done i'll post it here.

#13 Updated by Milan Stastny over 9 years ago

Got this done as a plugin(It is on a Plugin Page) so you are free to check it out.

We decided to do it by Roles, they are easier to modify and can be assigned per Project per User.
It Adds new Permission to the roles page Display in selection so when this is checked and there are some users in this role on that project, it shows in the list.

Also added Sellect All and Unsellect all buttons.

#14 Updated by Stéphane Gourichon about 9 years ago

  • +1.
  • Difference between Watcher Selection by Role and Watcher Selection by Group seems interesting. Thank you Milan.
  • Compatibility with versions above 1.0 is untested. Is there any risk in testing ? If trying features on a local copy of my Redmine 1.2 production setup seems to work, is there any risk in pushing that to actual production ?

#15 Updated by xianguo wei over 8 years ago

+1

#16 Updated by M. K. over 7 years ago

+1

#17 Updated by Arthur Zalevsky over 7 years ago

"almost 3 years". But feature is really worth to be done. So +1.

#18 Updated by HU an over 7 years ago

Badly need this great feature for big project! +1

#19 Updated by Maik Lindner over 7 years ago

+1 this feature would be really helpful!

#20 Updated by Fred Giusto over 7 years ago

+1 very useful

#21 Updated by Davy Tielens over 7 years ago

+1 It would really speed up our ticket creation.

#22 Updated by Yuu YAMASHITA over 7 years ago

+1

#23 Updated by Yuu YAMASHITA over 7 years ago

Watcher Selection by Group does not work expectedly on my installation of Redmine 2.2.

I wrote my edition of similar plugin which helps checking watchers by their group. Not yet well tested, but it's working on my Redmine 2.2.

http://www.redmine.org/plugins/watcher_groups

#24 Updated by Deoren Moor over 7 years ago

+1

#25 Updated by K. F. over 7 years ago

http://www.redmine.org/plugins/redmine_watcher_groups - in difference from other solutions, with this plugin notifications are sent to current group members.

#26 Updated by Dipan Mehta over 7 years ago

+1. This is necessary.

#27 Updated by Anonymous over 6 years ago

+1

#28 Updated by Eugene B over 6 years ago

+1

When I have to add 1-2 observers its ok. When I need to add a department which is ~10-15 users and is already at one group this becomes really useful.

#29 Updated by Vaclav Tůma over 6 years ago

K. F. wrote:

http://www.redmine.org/plugins/redmine_watcher_groups - in difference from other solutions, with this plugin notifications are sent to current group members.

Hi, is this plugin availabe (or will be available) for version 2.4.x ?

#30 Updated by Toshi MARUYAMA over 6 years ago

  • Related to Feature #4244: Multiple email addresses for each user added

#31 Updated by Stephane Lapie over 5 years ago

The above plug-in has one pitfall, it does not allow for proper "watcher" search queries : suppose I want to build an issue listing query for which tickets I am watching, from /issues/, at this stage, using this plug-in only shows issues where I (as a User) am explicitely set as a watcher, but does not take in account Groups.

I have made my own customized version of the watcher groups : https://github.com/darksoul42/redmine_watcher_groups which also :
  • works properly with Postgres, now (Removed the ` MySQL-ism from the SQL queries)
  • stores journal entries for watcher groups added/removed to ticket
  • has a Japanese locale
  • coordinates with the Redmine People plug-in from CRM to list group members

However, given it merely relies on the assumption that a Group and a User are deep down the same object, and the Principal->{User,Group} class inheritance to make things works, but basically shoves something in the watcher table that was not endorsed by Redmine via a raw SQL query, this means that for every Redmine upgrade, I have to be especially careful that this design axiom remains true. This sort of feels like a feature that should be built inside of Redmine core, and not as a plug-in.

I currently have a request from my company to implement a watcher query that takes in account watcher groups (which is of course very easy to do since this already works for tickets assigned to a group) :

--- app/models/query.rb.old    2014-10-21 12:21:55.601988870 +0900
+++ app/models/query.rb    2014-10-21 12:22:22.994264149 +0900
@@ -546,7 +546,7 @@
         if v.delete("me")
           if User.current.logged?
             v.push(User.current.id.to_s)
-            v += User.current.group_ids.map(&:to_s) if field == 'assigned_to_id'
+            v += User.current.group_ids.map(&:to_s) if field == 'assigned_to_id' or field == 'watcher_id'
           else
             v.push("0")
           end

Basically what this change does is, get the list of watchable issues from the watchers, when the watcher "user_id" is actually all the group_ids behind that user. This would not impact normal operation, as the Redmine current model does not allow addition via API of anything besides Users to the watchers table.

Given that this code change is located within a loop used to create the SQL query for the search, it can not realistically be implemented with a plugin :
  • Method override would mean playing catch-up with core code just to add a "or" condition here
  • Chaining alias are only really efficient for pre or post processing, and not usable here

#32 Updated by Akipii Oga over 5 years ago

+1

#33 Updated by Alexander Strunck over 5 years ago

Stephane Lapie wrote:

The above plug-in has one pitfall, it does not allow for proper "watcher" search queries : suppose I want to build an issue listing query for which tickets I am watching, from /issues/, at this stage, using this plug-in only shows issues where I (as a User) am explicitely set as a watcher, but does not take in account Groups.

I have made my own customized version of the watcher groups : https://github.com/darksoul42/redmine_watcher_groups which also :
  • works properly with Postgres, now (Removed the ` MySQL-ism from the SQL queries)
  • stores journal entries for watcher groups added/removed to ticket
  • has a Japanese locale
  • coordinates with the Redmine People plug-in from CRM to list group members

However, given it merely relies on the assumption that a Group and a User are deep down the same object, and the Principal->{User,Group} class inheritance to make things works, but basically shoves something in the watcher table that was not endorsed by Redmine via a raw SQL query, this means that for every Redmine upgrade, I have to be especially careful that this design axiom remains true. This sort of feels like a feature that should be built inside of Redmine core, and not as a plug-in.

I currently have a request from my company to implement a watcher query that takes in account watcher groups (which is of course very easy to do since this already works for tickets assigned to a group) :
[...]

Basically what this change does is, get the list of watchable issues from the watchers, when the watcher "user_id" is actually all the group_ids behind that user. This would not impact normal operation, as the Redmine current model does not allow addition via API of anything besides Users to the watchers table.

Given that this code change is located within a loop used to create the SQL query for the search, it can not realistically be implemented with a plugin :
  • Method override would mean playing catch-up with core code just to add a "or" condition here
  • Chaining alias are only really efficient for pre or post processing, and not usable here

I just installed the plugin. Why can I only assign a user group when the issue is already reported?

It would be nice if I can assign a group when I report a new issue.

#34 Updated by Martin G about 5 years ago

+1

#35 Updated by Benjamin Baumann about 5 years ago

If you are looking for a watcher group plugin working at issue update AND at issue creation you can take a look at Alexei Margasov fork of redmine_watcher_plugin : https://github.com/nauphone/redmine_watcher_groups

It's working in my redmine 2.6.1 install.

The group is not notified on the ticket creation though.

#36 Updated by Inese Ez almost 5 years ago

Benjamin Baumann wrote:

If you are looking for a watcher group plugin working at issue update AND at issue creation you can take a look at Alexei Margasov fork of redmine_watcher_plugin : https://github.com/nauphone/redmine_watcher_groups

It's working in my redmine 2.6.1 install.

as well as on 2.3.4.stable.

The group is not notified on the ticket creation though.

And all groups are visible/selectable without taking into account their restriction to certain projects (on issue creation and when adding watchers).

#37 Updated by Enderson Maia almost 5 years ago

Maybe someone could use the work done in chiliproject to back-port to Redmine.

See: https://www.chiliproject.org/issues/802

#38 Updated by Petr Mlčoch over 4 years ago

I make modifications to redmine_watcher_groups plugin to run it with Redmine 3.1.1.
See [[https://github.com/foton/redmine_watcher_groups.git]]

#39 Updated by Go MAEDA over 3 years ago

  • Duplicated by Feature #24943: Issue watchers to have groups in addition to users added

#40 Updated by Stephane Lapie about 3 years ago

Up, and a little update.
I updated upon Petr's plugin for Redmine 3.3.3-stable.

Here is my fork, but I also filled a pull request to his repository for good measure.
https://github.com/darksoul42/redmine_watcher_groups

It seems there is one aggravating edge case, as the acts_as_watchable code uses and expects a ActiveRecord_Associations_CollectionProxy (calls to the reset() method), and the current code degraded it as an array, I tried a quick and dirty hack.

Return the array, but with a method that takes in the initial object within its context, and calls the original reset() method.
Hopefully this does whatever cleanup is expected, but still functions properly.

I am more and more convinced that such a deep change should be handled internally by Redmine, instead of going through an arms race with plugins...

#41 Updated by Andreas Deininger about 3 years ago

Stephane Lapie wrote:

I updated upon Petr's plugin for Redmine 3.3.3-stable.

Stephane, thanks for your efforts.

Here is my fork, but I also filled a pull request to his repository for good measure.
https://github.com/darksoul42/redmine_watcher_groups

I installed your fork on a Redmine 3.4.0 instance, and it's causing problems there. Adding a watcher group work likes expected. Also, I can add an user as watcher (core Redmine functionality). If I try to delete the user I just added, Redmine server process exits silently. Maybe you can have a look into this?

#42 Updated by Stephane Lapie almost 3 years ago

Andreas,

I just had a look into that, and found that the ActiveRecord_Associations_CollectionProxy object does remain as such (and not degraded into an Array) when there are no watcher groups. Therefore in that case, I should not be defining the reset() method, which would trigger an endless recursion loop and a stack overflow. I suppose this is what you have seen, but could you confirm?

The fix is committed to my github.

#44 Updated by László Bokodi almost 3 years ago

Stephane,

Thanks for this great plugin. I installed on fresh 3.4.2 instance, but autocomplete function not worked for me.
I changed this line in watcher_groups_controller.rb from:

@groups = Group.active.like(params[:q]).find(:all, :limit => 100)

to:

@groups = Group.sorted.active.like(params[:q]).limit(100)

and autocomplete function started to work.

#45 Updated by Stephane Lapie almost 3 years ago

Ah, I see, this must have been an older API call I had missed.

I just integrated it.

Thanks!

#46 Updated by Andreas Deininger almost 3 years ago

Stephane Lapie wrote:

The fix is committed to my github.

Stephane, thanks for your efforts.

I suppose this is what you have seen, but could you confirm?

I just installed the master of your github project on an fresh Redmine 3.4.2 instance, and yes, I can confirm that the plugin is working now. That's great!

As far as I can see, there's still a small glitch here. Here is the way to reproduce:

  1. Open an existing ticket with no watchers defined.
  2. In the right bar, click on "Add" (right beneath the heading "Watchers group") to add a watcher group to an existing project. All persons of the watcher group are instantly shown as watchers. That's perfectly fine.
  3. In the right bar, click on "Add" (right beneath the heading "Watchers") to add a single person as watcher. The person is added as watcher, but does not show up instantly in the watchers list. If I reload the page, the person appears in the list of persons watching the issue.

The good thing is that the person is actually added, so it's merely a display issue. Nevertheless, this behaviour might confuse users.

Note: The same applies if I delete an existing watcher (single person) by clicking on the trash bin symbol. The person is only removed from the watchers list after reloading the page.

#47 Updated by cleber souza about 2 years ago

Stephane Lapie wrote:

Up, and a little update.
I updated upon Petr's plugin for Redmine 3.3.3-stable.

Here is my fork, but I also filled a pull request to his repository for good measure.
https://github.com/darksoul42/redmine_watcher_groups

It seems there is one aggravating edge case, as the acts_as_watchable code uses and expects a ActiveRecord_Associations_CollectionProxy (calls to the reset() method), and the current code degraded it as an array, I tried a quick and dirty hack.

Return the array, but with a method that takes in the initial object within its context, and calls the original reset() method.
Hopefully this does whatever cleanup is expected, but still functions properly.

I am more and more convinced that such a deep change should be handled internally by Redmine, instead of going through an arms race with plugins...

Thanks Stephane for this version of the plugin, but in the version of Redmine 3.4.4 stable. The Plugin is installed normally, but when it will access the task screen the error below is displayed.

#48 Updated by cleber souza about 2 years ago

cleber souza wrote:

Stephane Lapie wrote:

Up, and a little update.
I updated upon Petr's plugin for Redmine 3.3.3-stable.

Here is my fork, but I also filled a pull request to his repository for good measure.
https://github.com/darksoul42/redmine_watcher_groups

It seems there is one aggravating edge case, as the acts_as_watchable code uses and expects a ActiveRecord_Associations_CollectionProxy (calls to the reset() method), and the current code degraded it as an array, I tried a quick and dirty hack.

Return the array, but with a method that takes in the initial object within its context, and calls the original reset() method.
Hopefully this does whatever cleanup is expected, but still functions properly.

I am more and more convinced that such a deep change should be handled internally by Redmine, instead of going through an arms race with plugins...

Thanks Stephane for this version of the plugin, but in the version of Redmine 3.4.4 stable. The Plugin is installed normally, but when it will access the task screen the error below is displayed.

Resolved compatibility version: https://github.com/piccio/redmine_watchers_groups

#49 Updated by Vlad Belkov about 2 years ago

+1

#50 Updated by Go MAEDA almost 2 years ago

  • Duplicated by Feature #15164: Make it possible to add group as a watcher added

#51 Updated by Go MAEDA almost 2 years ago

  • Related to Feature #29501: Allow addition of watcher group via bulk edit context menu added

#52 Updated by Sunding Wei over 1 year ago

cleber souza wrote:
...
Resolved compatibility version: https://github.com/piccio/redmine_watchers_groups

Hi Cleber Souza

I installed the plugin you provided for Redmine v3.4.4.stable, it works for creating issues with groups watchers, but when I open the issue I just created, the ruby process takes CPU 100%...I have to uninstall it, do you know what should be the issue?

#53 Updated by Sunding Wei over 1 year ago

I fixed the CPU 100% issue of Cleber Souza codes

Redmine 3.4.4.stable
Original: https://github.com/piccio/redmine_watchers_groups

Fixes

diff --git a/lib/redmine_watchers_groups/group_patch.rb b/lib/redmine_watchers_groups/group_patch.rb
index 567f2a0..a3a7bb8 100644
--- a/lib/redmine_watchers_groups/group_patch.rb
+++ b/lib/redmine_watchers_groups/group_patch.rb
@@ -22,7 +22,17 @@ module RedmineWatchersGroups

         unless watcher_ids.empty?
           # simple combinations of watchers
-          simple_watchers_combinations = watcher_ids.repeated_combination(watcher_ids.length).to_a.map(&:uniq).uniq
+          # FIXME: CPU 100%, producing too many combinations
+          # simple_watchers_combinations = watcher_ids.repeated_combination(watcher_ids.length).to_a.map(&:uniq).uniq
+         users = User.where(id: watcher_ids)
+         dict = {}
+         users.each do |user|
+           user.group_ids.each do |gid|
+              ids = dict[gid].to_a.push(user.id)
+              dict[gid] = ids.uniq
+            end
+          end
+         simple_watchers_combinations = dict.values

           simple_watchers_combinations.each do |simple_watchers_combination|
             # the first query find all groups that contains all the current watchers as members through
@@ -48,4 +58,5 @@ module RedmineWatchersGroups
     end

   end
-end
\ No newline at end of file
+end
+

#54 Updated by Jonas De Meulenaere over 1 year ago

+1 to have it implemented as a standard feature

#55 Updated by Thomas Guiot over 1 year ago

Sunding Wei wrote:

I fixed the CPU 100% issue of Cleber Souza codes

Redmine 3.4.4.stable
Original: https://github.com/piccio/redmine_watchers_groups

Fixes
[...]

This plugin crashed the tickets where I used a group as observer. see https://github.com/piccio/redmine_watchers_groups/issues/4

and +1 to have it as a standard feature

#56 Updated by Gerhard Ferdan about 1 year ago

+1 to have it implemented as a standard feature.
We used the https://www.redmine.org/plugins/rmp_group_watchers plugin until now but since updating to V4.x the plugin isn't longer useable

#57 Updated by Taine Woo 12 months ago

+1

#58 Updated by Yuichi HARADA 5 months ago

Fixed functionality to allow adding groups to the watcher of issues.
I attached a patch.

#59 Updated by Go MAEDA 5 months ago

Yuichi HARADA wrote:

Fixed functionality to allow adding groups to the watcher of issues.
I attached a patch.

Thank you for posting the patch for this long-awaited feature. I tried out the patch and it works fine.

I slightly updated the patch to fix RuboCop offenses.

#60 Updated by Go MAEDA 5 months ago

  • Target version changed from Candidate for next major release to 4.2.0

Setting the target version to 4.2.0.

#61 Updated by Yuichi HARADA 5 months ago

Sorry, When entering a new issue, the groups of the project members must appear in the Watchers field.
Fixed with the following patch.

diff --git a/app/controllers/watchers_controller.rb b/app/controllers/watchers_controller.rb
index c741a9326..e0de27464 100644
--- a/app/controllers/watchers_controller.rb
+++ b/app/controllers/watchers_controller.rb
@@ -122,14 +122,16 @@ class WatchersController < ApplicationController
   end

   def users_for_new_watcher
-    scope = nil
+    scope, scope_groups = nil
     if params[:q].blank? && @project.present?
       scope = @project.users
+      scope_groups = @project.principals.merge(Group.givable)
     else
       scope = User.all.limit(100)
+      scope_groups = Group.givable.limit(100)
     end
     users = scope.active.visible.sorted.like(params[:q]).to_a
-    users += Group.givable.active.visible.sorted.like(params[:q]).to_a
+    users += scope_groups.active.visible.sorted.like(params[:q]).to_a
     if @watchables && @watchables.size == 1
       users -= @watchables.first.watcher_users
     end
diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb
index 34411470f..0f327b77e 100644
--- a/app/helpers/issues_helper.rb
+++ b/app/helpers/issues_helper.rb
@@ -365,8 +365,11 @@ module IssuesHelper
   # on the new issue form
   def users_for_new_issue_watchers(issue)
     users = issue.watcher_users.select{|u| u.status == User::STATUS_ACTIVE}
-    if issue.project.users.count + Group.givable.count <= 20
-      users = (users + issue.project.users.sort + Group.givable.sort).uniq
+    project = issue.project
+    scope_users = project.users
+    scope_groups = project.principals.merge(Group.givable)
+    if scope_users.count + scope_groups.count <= 20
+      users = (users + scope_users.sort + scope_groups.sort).uniq
     end
     users
   end
diff --git a/lib/plugins/acts_as_watchable/lib/acts_as_watchable.rb b/lib/plugins/acts_as_watchable/lib/acts_as_watchable.rb
index e8dc9ee7b..e37151d8f 100644
--- a/lib/plugins/acts_as_watchable/lib/acts_as_watchable.rb
+++ b/lib/plugins/acts_as_watchable/lib/acts_as_watchable.rb
@@ -31,7 +31,7 @@ module Redmine

         # Returns an array of users that are proposed as watchers
         def addable_watcher_users
-          users = (self.project.users.sort + Group.givable.sort) - self.watcher_users
+          users = (self.project.users.sort + self.project.principals.merge(Group.givable).sort) - self.watcher_users
           if respond_to?(:visible?)
             users.reject! {|user| user.is_a?(User) && !visible?(user)}
           end

#62 Updated by Go MAEDA 5 months ago

  • Subject changed from Allowing to add user groups as watchers for issues to Allow adding user groups as watchers for issues
  • Status changed from New to Closed
  • Assignee set to Go MAEDA
  • Resolution set to Fixed

Committed the patch. Thank you for writing the patch for this long-awaited feature.

#63 Updated by Go MAEDA 5 months ago

  • Related to Patch #33036: Add missing fixture to IssuesControllerTest added

#64 Updated by Go MAEDA 4 months ago

  • Related to Patch #33100: Fix a test - Issue watchers are not always sorted by id added

#65 Updated by Marius BALTEANU 3 months ago

  • File 0003-Use-principals-in-acts_as_watchable.patch added
  • File 0002-Use-Principal-to-get-users-and-groups-for-watchers-i.patch added
  • File 0001-Get-the-list-of-new-issue-watchers-using-single-quer.patch added
  • File 0004-Use-scope-assignable_watchers.patch added
  • Status changed from Closed to Reopened

I'm reopening this in order to fix some possible performance issues generated by these changes or some unnecessary queries:

1. Removing preload(:email_address) from watchers_list (app/helpers/watchers_helper.rb) generates N+1. For each watcher, it will be triggered a query to take the email address. Below an example of an issue with 5 watchers:

D, [2020-04-20T20:26:37.979524 #14] DEBUG -- :   User Load (0.9ms)  SELECT `users`.* FROM `users` INNER JOIN `watchers` ON `users`.`id` = `watchers`.`user_id` WHERE `users`.`type` IN ('User', 'AnonymousUser') AND `watchers`.`watchable_id` = 2 AND `watchers`.`watchable_type` = 'Issue'
D, [2020-04-20T20:26:37.982628 #14] DEBUG -- :   EmailAddress Load (0.6ms)  SELECT  `email_addresses`.* FROM `email_addresses` WHERE `email_addresses`.`user_id` = 1 AND `email_addresses`.`is_default` = TRUE LIMIT 1
D, [2020-04-20T20:26:37.988894 #14] DEBUG -- :   CACHE EmailAddress Load (0.1ms)  SELECT  `email_addresses`.* FROM `email_addresses` WHERE `email_addresses`.`user_id` = 3 AND `email_addresses`.`is_default` = TRUE LIMIT 1
D, [2020-04-20T20:26:37.991870 #14] DEBUG -- :   EmailAddress Load (0.6ms)  SELECT  `email_addresses`.* FROM `email_addresses` WHERE `email_addresses`.`user_id` = 4 AND `email_addresses`.`is_default` = TRUE LIMIT 1
D, [2020-04-20T20:26:37.994146 #14] DEBUG -- :   CACHE EmailAddress Load (0.0ms)  SELECT  `email_addresses`.* FROM `email_addresses` WHERE `email_addresses`.`user_id` = 2 AND `email_addresses`.`is_default` = TRUE LIMIT 1
D, [2020-04-20T20:26:37.996693 #14] DEBUG -- :   EmailAddress Load (0.6ms)  SELECT  `email_addresses`.* FROM `email_addresses` WHERE `email_addresses`.`user_id` = 8 AND `email_addresses`.`is_default` = TRUE LIMIT 1
I, [2020-04-20T20:26:38.000373 #14]  INFO -- :   Rendered watchers/_watchers.html.erb (92.5ms)
I, [2020-04-20T20:26:38.006492 #14]  INFO -- :   Rendered issues/show.html.erb within layouts/base (1418.9ms)
D, [2020-04-20T20:26:38.010703 #14] DEBUG -- :   Setting Load (0.6ms)  SELECT  `settings`.* FROM `settings` WHERE `settings`.`name` = 'app_title' ORDER BY `settings`.`id` DESC LIMIT 1
D, [2020-04-20T20:26:38.028354 #14] DEBUG -- :   CACHE EmailAddress Load (0.0ms)  SELECT  `email_addresses`.* FROM `email_addresses` WHERE `email_addresses`.`user_id` = 1 AND `email_addresses`.`is_default` = TRUE LIMIT 1

I think we should find a better way to retrieve the watchers in order to avoid all those queries.

2. In app/helpers/issues_helper.rb we can avoid querying the database twice to get the users and then the groups by using the patch 0001-Get-the-list-of-new-issue-watchers-using-single-quer.patch. Also, the patch limits the results returned from the database to 20 because that is the maximum number of watchers displayed in the new issue form.

3. Same as 2 but in watchers_controller and acts_as_watchable with patches 0002-Use-Principal-to-get-users-and-groups-for-watchers-i.patch and 0003-Use-principals-in-acts_as_watchable.patch

Example:

D, [2020-04-20T21:05:29.355800 #14] DEBUG -- :   User Load (0.7ms)  SELECT `users`.* FROM `users` WHERE `users`.`type` IN ('User', 'AnonymousUser') AND `users`.`status` = 1 AND `users`.`id` IN (3, 2)
D, [2020-04-20T21:05:29.358191 #14] DEBUG -- :  Group Load (0.7ms)  SELECT `users`.* FROM `users` WHERE `users`.`type` IN ('Group', 'GroupBuiltin', 'GroupAnonymous', 'GroupNonMember') AND `users`.`type` = 'Group' AND `users`.`status` = 1 AND `users`.`id` IN (3, 2)

After:

D, [2020-04-20T21:09:11.148277 #14] DEBUG -- :   Principal Load (0.9ms)  SELECT `users`.* FROM `users` WHERE `users`.`status` = 1 AND `users`.`id` IN (3, 2) AND `users`.`type` IN ('User', 'Group')

4. This patch adds a new scope named assignable_watchers to Principal in order to avoid duplicated logic across multiple files. Any feedback is welcome.

#66 Updated by Marius BALTEANU 3 months ago

  • File deleted (0004-Use-scope-assignable_watchers.patch)

#67 Updated by Marius BALTEANU 3 months ago

  • File 0004-Use-scope-assignable_watchers.patch added

Fixed a minor issue.

Tests pass.

#68 Updated by Go MAEDA 3 months ago

Marius, thank you for improving the code but I noticed that 0001-Get-the-list-of-new-issue-watchers-using-single-quer.patch change the behavior.

Redmine 2.5 or later does not show watchers checkboxes on the new issue page when more than 20 members belong to the project (#8562). However, after applying the patch, first 20 members/groups are displayed even when the number of members exceeds 20.

#69 Updated by Marius BALTEANU 3 months ago

  • File deleted (0004-Use-scope-assignable_watchers.patch)

#70 Updated by Marius BALTEANU 3 months ago

  • File deleted (0002-Use-Principal-to-get-users-and-groups-for-watchers-i.patch)

#71 Updated by Marius BALTEANU 3 months ago

  • File deleted (0001-Get-the-list-of-new-issue-watchers-using-single-quer.patch)

#72 Updated by Marius BALTEANU 3 months ago

  • File deleted (0003-Use-principals-in-acts_as_watchable.patch)

#73 Updated by Marius BALTEANU 3 months ago

Go MAEDA wrote:

Marius, thank you for improving the code but I noticed that 0001-Get-the-list-of-new-issue-watchers-using-single-quer.patch change the behavior.

Redmine 2.5 or later does not show watchers checkboxes on the new issue page when more than 20 members belong to the project (#8562). However, after applying the patch, first 20 members/groups are displayed even when the number of members exceeds 20.

You're right, thanks for pointing this out, I've updated the patch series to fix this.

#74 Updated by Go MAEDA 3 months ago

  • Status changed from Reopened to Closed

Committed the patches attached in #4511#note-73. Thank you.

#75 Updated by Marius BALTEANU 3 months ago

Regarding first problem (N+1), we don’t do anything?

#76 Updated by Taner Tas about 1 month ago

I just need to add groups as watchers like regular user. I tried patches shared in this issue but none of them are applicable for 4.0 version.

if subject.present?
    self.add_watcher(project.default_assigned_to) unless watched_by?(project.default_assigned_to)
end

Our project default assignees are groups. The custom workflow code above doesn't work because, Redmine 4.0 doesn't accept groups as known type for watchers.

Also available in: Atom PDF