Defect #37958

Groups added to watchers are not shown as links

Added by Miodrag Milic 2 months ago. Updated about 1 month ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Marius BALTEANU% Done:

0%

Category:Issues
Target version:5.0.4
Resolution:Fixed Affected version:5.0.3

Description

After #4511 (v4.2.0), its possible to add Redmine groups as watchers on issues.

Users are shown as links, and groups are shown in Overview as links, and in Assignee field. However, gruops are shown as text when they are added to watchers

In the screenshot bellow, you can see 2 groups - 1 in assigne and 1 in watchers. You can see that user watcher is provided as link, while group is text

This behavior is not consistent and its harder for people involved to see group members (they must go to Overview page and search for that group there).

Untitled.png (147 KB) Miodrag Milic, 2022-11-23 11:52

Associated revisions

Revision 21967
Added by Marius BALTEANU 2 months ago

Fix groups added to watchers are not shown as links (#37958).

Patch by Mizuki ISHIKAWA.

Revision 21970
Added by Marius BALTEANU 2 months ago

Merge r21967 from trunk to 5.0-stable (#37958).

History

#1 Updated by Mizuki ISHIKAWA 2 months ago

I think the following code will solve the problem.

diff --git a/app/helpers/watchers_helper.rb b/app/helpers/watchers_helper.rb
index fc97da1056..97067378e4 100644
--- a/app/helpers/watchers_helper.rb
+++ b/app/helpers/watchers_helper.rb
@@ -51,7 +51,7 @@ module WatchersHelper
     lis = object.watcher_users.collect do |user|
       s = ''.html_safe
       s << avatar(user, :size => "16").to_s
-      s << link_to_user(user, :class => 'user')
+      s << link_to_principal(user, class: user.class.to_s.downcase)
       if object.respond_to?(:visible?) && user.is_a?(User) && !object.visible?(user)
         s << content_tag('span', l(:notice_invalid_watcher), class: 'icon-only icon-warning', title: l(:notice_invalid_watcher))
       end
diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb
index a10abbd129..58e51348a9 100644
--- a/test/functional/issues_controller_test.rb
+++ b/test/functional/issues_controller_test.rb
@@ -2726,7 +2726,7 @@ class IssuesControllerTest < Redmine::ControllerTest
       end
       assert_select "li.user-10" do
         assert_select 'img.gravatar[title=?]', 'A Team'
-        assert_select 'a[href="/users/10"]', false
+        assert_select 'a[href="/groups/10"]'
         assert_select 'a[class*=delete]'
       end
     end

#2 Updated by Dmitry Makurin 2 months ago

The problem is that there are multiple methods to render user/group links:
  • link_to_user
  • link_to_principal
  • link_to_group

Before r21073 only link_to_(user/group) existed and it was clear what method was responsible for.

link_to_principal combined both link_to_user and link_to_group and now it's really confusing what the difference between all of them.

I would suggest to unify link_to_user and link_to_group into link_to_principal or properly decompose link_to_principal.

#3 Updated by Marius BALTEANU 2 months ago

  • Status changed from New to Resolved
  • Assignee set to Marius BALTEANU
  • Target version set to 5.0.4
  • Resolution set to Fixed

I've committed for now the patch posted by Mizuki to catch the release of new maintenance version.

#4 Updated by Marius BALTEANU 2 months ago

Dmitry Makurin wrote:

The problem is that there are multiple methods to render user/group links:
  • link_to_user
  • link_to_principal
  • link_to_group

Before r21073 only link_to_(user/group) existed and it was clear what method was responsible for.

link_to_principal combined both link_to_user and link_to_group and now it's really confusing what the difference between all of them.

I would suggest to unify link_to_user and link_to_group into link_to_principal or properly decompose link_to_principal.

Indeed, it sounds confusing. If you have code proposal, I'll very happy to review it.

#5 Updated by Marius BALTEANU about 1 month ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF