FIX: Tag watching for everyone tag groups (#15622)

* FIX: Tag watching for everyone tag groups

Tags in tag groups that have permissions set to everyone were not able
to be saved correctly. A user on their preferences page would mark the
tags that they wanted to save, but the watched_tags in the response
would be empty. This did not apply to admins, just regular users. Even
though the watched tags were being saved in the db, the user serializer
response was filtering them out. When a user refreshed their preferences
pages it would show zero watched tags.

This appears to be a regression introduced by:

0f598ca51e

The issue that needed to be fixed is that we don't track the "everyone"
group (which has an id of 0) in the group_users table. This is because
everyone has access to it, so why fill a row for every single user, that
would be a lot. The fix was to update the query to include tag groups
that had permissions set to the "everyone" group (group_id 0).

I also added another check to the existing spec for updating
watched tags for tags that aren't in a tag group so that it checks the
response body. I then added a new spec which updates watched tags for
tags in a tag group which has permissions set to everyone.

* Resolve failing tests

Improve SQL query syntax for including the "everyone" group with the id
of 0.

This commit also fixes a few failing tests that were introduced. It
turns out that the Fabrication of the Tag Group Permissions was faulty.
What happens when creating the tag groups without any permissions is
that it sets the permission to "everyone". If we then follow up with
fabricating a tag group permission on the tag group instead of having a
single permission it will have 2 (everyone + the group specified)! We
don't want this. To fix it I removed the fabrication of tag group
permissions and just set the permissions directly when creating the tag
group.

* Use response.parsed_body instead of JSON.parse
This commit is contained in:
Blake Erickson 2022-01-18 15:02:29 -07:00 committed by GitHub
parent bb1eacf184
commit 12f041de5d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 23 additions and 11 deletions

View File

@ -11,7 +11,7 @@ class TagUser < ActiveRecord::Base
.joins("LEFT OUTER JOIN tag_group_permissions ON tag_group_memberships.tag_group_id = tag_group_permissions.tag_group_id")
.joins("LEFT OUTER JOIN group_users on group_users.user_id = tag_users.user_id")
.where("(tag_group_permissions.group_id IS NULL
OR tag_group_permissions.group_id = group_users.group_id
OR tag_group_permissions.group_id IN (0, group_users.group_id)
OR group_users.group_id = :staff_group_id)
AND tag_users.notification_level IN (:notification_levels)",
staff_group_id: Group::AUTO_GROUPS[:staff],

View File

@ -40,12 +40,10 @@ describe TagUser do
it "scopes to notification levels visible by tag group permission" do
group1 = Fabricate(:group)
tag_group1 = Fabricate(:tag_group, tags: [tag1])
Fabricate(:tag_group_permission, tag_group: tag_group1, group: group1)
tag_group1 = Fabricate(:tag_group, tags: [tag1], permissions: { group1.name => 1 })
group2 = Fabricate(:group)
tag_group2 = Fabricate(:tag_group, tags: [tag2])
Fabricate(:tag_group_permission, tag_group: tag_group2, group: group2)
tag_group2 = Fabricate(:tag_group, tags: [tag2], permissions: { group2.name => 1 })
Fabricate(:group_user, group: group1, user: user1)
@ -56,8 +54,7 @@ describe TagUser do
it "scopes to notification levels visible because user is staff" do
group2 = Fabricate(:group)
tag_group2 = Fabricate(:tag_group, tags: [tag2])
Fabricate(:tag_group_permission, tag_group: tag_group2, group: group2)
tag_group2 = Fabricate(:tag_group, tags: [tag2], permissions: { group2.name => 1 })
staff_group = Group.find(Group::AUTO_GROUPS[:staff])
Fabricate(:group_user, group: staff_group, user: user1)
@ -330,8 +327,7 @@ describe TagUser do
it "does not show a tag is tracked if the user does not belong to the tag group with permissions" do
group = Fabricate(:group)
tag_group = Fabricate(:tag_group, tags: [tag2])
Fabricate(:tag_group_permission, tag_group: tag_group, group: group)
tag_group = Fabricate(:tag_group, tags: [tag2], permissions: { group.name => 1 })
expect(TagUser.notification_levels_for(user).keys).to match_array([tag1.name, tag3.name, tag4.name])
end

View File

@ -1972,6 +1972,7 @@ describe UsersController do
}
expect(response.status).to eq(200)
expect(response.parsed_body['user']['watched_tags'].count).to eq(2)
user.reload
@ -2000,6 +2001,22 @@ describe UsersController do
expect(user.card_background_upload).to eq(upload)
end
it 'updates watched tags in everyone tag group' do
SiteSetting.tagging_enabled = true
tags = [Fabricate(:tag), Fabricate(:tag)]
group = Fabricate(:group, name: 'group', mentionable_level: Group::ALIAS_LEVELS[:everyone])
tag_group = Fabricate(:tag_group, tags: tags)
Fabricate(:tag_group_permission, tag_group: tag_group, group: group)
tag_synonym = Fabricate(:tag, target_tag: tags[1])
put "/u/#{user.username}.json", params: {
watched_tags: "#{tags[0].name},#{tag_synonym.name}"
}
expect(response.status).to eq(200)
expect(response.parsed_body['user']['watched_tags'].count).to eq(2)
end
context 'a locale is chosen that differs from I18n.locale' do
before do
SiteSetting.allow_user_locale = true

View File

@ -1429,8 +1429,7 @@ describe PostAlerter do
end
it "does not notify a user watching a tag with tag group permissions that he does not belong to" do
tag_group = Fabricate(:tag_group, tags: [tag])
Fabricate(:tag_group_permission, tag_group: tag_group, group: group)
tag_group = Fabricate(:tag_group, tags: [tag], permissions: { group.name => 1 })
TagUser.change(user.id, tag.id, TagUser.notification_levels[notification_level])