Feature #42630 » 0001-Show-reaction-counts-and-user-names-only-for-reactions-visible-to-the-logged-in-user.patch
| app/models/reaction.rb | ||
|---|---|---|
| 25 | 25 | |
| 26 | 26 |
scope :by, ->(user) { where(user: user) }
|
| 27 | 27 |
scope :for_reactable, ->(reactable) { where(reactable: reactable) }
|
| 28 |
scope :visible, ->(user) { where(user: User.visible(user)) }
|
|
| 28 | 29 | |
| 29 | 30 |
# Represents reaction details for a reactable object |
| 30 | 31 |
Detail = Struct.new( |
| 31 |
# Total number of reactions |
|
| 32 |
:reaction_count, |
|
| 33 | 32 |
# Users who reacted and are visible to the target user |
| 34 | 33 |
:visible_users, |
| 35 | 34 |
# Reaction of the target user |
| 36 | 35 |
:user_reaction |
| 37 | 36 |
) do |
| 38 |
def initialize(reaction_count: 0, visible_users: [], user_reaction: nil)
|
|
| 37 |
def initialize(visible_users: [], user_reaction: nil) |
|
| 39 | 38 |
super |
| 40 | 39 |
end |
| 40 | ||
| 41 |
def reaction_count = visible_users.size |
|
| 41 | 42 |
end |
| 42 | 43 | |
| 43 | 44 |
def self.build_detail_map_for(reactables, user) |
| 44 |
reactions = preload(:user)
|
|
| 45 |
reactions = visible(user)
|
|
| 45 | 46 |
.for_reactable(reactables) |
| 47 |
.preload(:user) |
|
| 46 | 48 |
.select(:id, :reactable_id, :user_id) |
| 47 | 49 |
.order(id: :desc) |
| 48 | 50 | |
| 49 |
# Prepare IDs of users who reacted and are visible to the user |
|
| 50 |
visible_user_ids = User.visible(user) |
|
| 51 |
.joins(:reactions) |
|
| 52 |
.where(reactions: for_reactable(reactables)) |
|
| 53 |
.pluck(:id).to_set |
|
| 54 | ||
| 55 | 51 |
reactions.each_with_object({}) do |reaction, m|
|
| 56 | 52 |
m[reaction.reactable_id] ||= Detail.new |
| 57 | 53 | |
| 58 | 54 |
m[reaction.reactable_id].then do |detail| |
| 59 |
detail.reaction_count += 1 |
|
| 60 |
detail.visible_users << reaction.user if visible_user_ids.include?(reaction.user.id) |
|
| 55 |
detail.visible_users << reaction.user |
|
| 61 | 56 |
detail.user_reaction = reaction if reaction.user == user |
| 62 | 57 |
end |
| 63 | 58 |
end |
| test/functional/issues_controller_test.rb | ||
|---|---|---|
| 3347 | 3347 |
# The current_user can only see members who belong to projects that the current_user has access to. |
| 3348 | 3348 |
# Since the Redmine Admin user does not belong to any projects visible to the current_user, |
| 3349 | 3349 |
# the Redmine Admin user's name is not displayed in the reaction user list. Instead, "1 other" is shown. |
| 3350 |
assert_select 'a.reaction-button[title=?]', 'Dave Lopper, John Smith, and 1 other' do
|
|
| 3351 |
assert_select 'span.icon-label', '3'
|
|
| 3350 |
assert_select 'a.reaction-button[title=?]', 'Dave Lopper and John Smith' do
|
|
| 3351 |
assert_select 'span.icon-label', '2'
|
|
| 3352 | 3352 |
end |
| 3353 | 3353 |
end |
| 3354 | 3354 | |
| test/helpers/reactions_helper_test.rb | ||
|---|---|---|
| 106 | 106 |
assert_select_in result, 'a.reaction-button[title=?]', expected_tooltip |
| 107 | 107 |
end |
| 108 | 108 | |
| 109 |
test 'reaction_button displays non-visible users as "X other" in the tooltip' do
|
|
| 109 |
test 'reaction_button should not count and display non-visible users' do
|
|
| 110 | 110 |
issue2 = issues(:issues_002) |
| 111 | 111 | |
| 112 | 112 |
issue2.reaction_detail = Reaction::Detail.new( |
| 113 |
# The remaining 3 users are non-visible users |
|
| 114 |
reaction_count: 5, |
|
| 115 | 113 |
visible_users: users(:users_002, :users_003) |
| 116 | 114 |
) |
| 117 | 115 | |
| ... | ... | |
| 119 | 117 |
reaction_button(issue2) |
| 120 | 118 |
end |
| 121 | 119 | |
| 122 |
assert_select_in result, 'a.reaction-button[title=?]', 'John Smith, Dave Lopper, and 3 others'
|
|
| 120 |
assert_select_in result, 'a.reaction-button[title=?]', 'John Smith and Dave Lopper'
|
|
| 123 | 121 | |
| 124 | 122 |
# When all users are non-visible users |
| 125 | 123 |
issue2.reaction_detail = Reaction::Detail.new( |
| 126 |
reaction_count: 2, |
|
| 127 | 124 |
visible_users: [] |
| 128 | 125 |
) |
| 129 | 126 | |
| ... | ... | |
| 131 | 128 |
reaction_button(issue2) |
| 132 | 129 |
end |
| 133 | 130 | |
| 134 |
assert_select_in result, 'a.reaction-button[title=?]', '2 others' |
|
| 131 |
assert_select_in result, 'a.reaction-button[title]', false |
|
| 132 |
assert_select_in result, 'a.reaction-button' do |
|
| 133 |
assert_select 'span.icon-label', '0' |
|
| 134 |
end |
|
| 135 | 135 |
end |
| 136 | 136 | |
| 137 | 137 |
test 'reaction_button formats the tooltip content based on the support.array settings of each locale' do |
| test/unit/lib/redmine/reaction_test.rb | ||
|---|---|---|
| 42 | 42 |
Issue.preload_reaction_details([issue1, issue2]) |
| 43 | 43 | |
| 44 | 44 |
expected_issue1_reaction_detail = Reaction::Detail.new( |
| 45 |
reaction_count: 3, |
|
| 46 | 45 |
visible_users: [users(:users_003), users(:users_002), users(:users_001)], |
| 47 | 46 |
user_reaction: reactions(:reaction_002) |
| 48 | 47 |
) |
| ... | ... | |
| 53 | 52 | |
| 54 | 53 |
# Even when an object has no reactions, an empty ReactionDetail is set. |
| 55 | 54 |
assert_equal Reaction::Detail.new( |
| 56 |
reaction_count: 0, |
|
| 57 | 55 |
visible_users: [], |
| 58 | 56 |
user_reaction: nil |
| 59 | 57 |
), issue2.reaction_detail |
| ... | ... | |
| 107 | 105 |
assert_nil message7.instance_variable_get(:@reaction_detail) |
| 108 | 106 | |
| 109 | 107 |
assert_equal Reaction::Detail.new( |
| 110 |
reaction_count: 1, |
|
| 111 | 108 |
visible_users: [users(:users_002)], |
| 112 | 109 |
user_reaction: reactions(:reaction_009) |
| 113 | 110 |
), message7.reaction_detail |
| ... | ... | |
| 122 | 119 |
comment1.load_reaction_detail |
| 123 | 120 | |
| 124 | 121 |
assert_equal Reaction::Detail.new( |
| 125 |
reaction_count: 1, |
|
| 126 | 122 |
visible_users: [users(:users_002)], |
| 127 | 123 |
user_reaction: nil |
| 128 | 124 |
), comment1.reaction_detail |
| test/unit/reaction_test.rb | ||
|---|---|---|
| 75 | 75 | |
| 76 | 76 |
expected = {
|
| 77 | 77 |
1 => Reaction::Detail.new( |
| 78 |
reaction_count: 3, |
|
| 79 | 78 |
visible_users: [users(:users_003), users(:users_002), users(:users_001)], |
| 80 | 79 |
user_reaction: reactions(:reaction_003) |
| 81 | 80 |
), |
| 82 | 81 |
6 => Reaction::Detail.new( |
| 83 |
reaction_count: 1, |
|
| 84 | 82 |
visible_users: [users(:users_002)], |
| 85 | 83 |
user_reaction: nil |
| 86 | 84 |
) |
