FIX: users watching tags in open tag groups not notified (#16384)

All users are members of the EVERYONE group, but this group is special and
is omitted from the group_users table. When checking permission we need to
make sure we also add a bypass.

This also fixes a very buggy test in post_alerter, it was confirming the
broken behavior due to fabricator flow.

When it defined the tag group the everyone group automatically had full access
then the additional permission fabricated just added one more group. After
fix was made to code the test started failing. Fabricators can be risky.
This commit is contained in:
Sam 2022-04-06 11:43:57 +10:00 committed by GitHub
parent 750fab0d52
commit 1598e6b489
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 17 additions and 4 deletions

View File

@ -802,7 +802,12 @@ class PostAlerter
LEFT JOIN tag_group_memberships tgm ON tag_users.tag_id = tgm.tag_id
LEFT JOIN tag_group_permissions tgp ON tgm.tag_group_id = tgp.tag_group_id
LEFT JOIN group_users gu ON gu.user_id = tag_users.user_id
WHERE (tgp.group_id IS NULL OR tgp.group_id = gu.group_id OR gu.group_id = :staff_group_id)
WHERE (
tgp.group_id IS NULL OR
tgp.group_id = gu.group_id OR
tgp.group_id = :everyone_group_id OR
gu.group_id = :staff_group_id
)
AND (tag_users.notification_level = :watching
AND tag_users.tag_id IN (:tag_ids)
AND (tu.user_id IS NULL OR tu.notification_level = :watching))
@ -814,7 +819,8 @@ class PostAlerter
topic_id: post.topic_id,
category_id: post.topic.category_id,
tag_ids: tag_ids,
staff_group_id: Group::AUTO_GROUPS[:staff]
staff_group_id: Group::AUTO_GROUPS[:staff],
everyone_group_id: Group::AUTO_GROUPS[:everyone]
)
if group_ids.present?

View File

@ -183,6 +183,11 @@ describe TagUser do
it "sets notification levels correctly" do
# define a wide open tag group to ensure it also works
group = TagGroup.new(name: 'Visible & usable by everyone', tag_names: [watched_tag.name])
group.permissions = [[Group::AUTO_GROUPS[:everyone], TagGroupPermission.permission_types[:full]]]
group.save!
expect(Notification.where(user_id: user.id, topic_id: watched_post.topic_id).count).to eq 1
expect(Notification.where(user_id: user.id, topic_id: tracked_post.topic_id).count).to eq 0

View File

@ -1372,8 +1372,10 @@ describe PostAlerter do
tag = Fabricate(:tag)
topic = Fabricate(:topic, tags: [tag])
post = Fabricate(:post, topic: topic)
tag_group = Fabricate(:tag_group, tags: [tag])
Fabricate(:tag_group_permission, tag_group: tag_group, group: group)
tag_group = TagGroup.new(name: 'Only visible to group', tag_names: [tag.name])
tag_group.permissions = [[group.id, TagGroupPermission.permission_types[:full]]]
tag_group.save!
TagUser.change(user.id, tag.id, TagUser.notification_levels[:watching])