FIX: indirectly muted categories for topic-tracking-state (#16067)

Topics belonging to indirectly muted categories should be excluded from topic-tracking-state report.
This commit is contained in:
Krzysztof Kotlarek 2022-03-02 15:02:09 +11:00 committed by GitHub
parent 9d2c5c489e
commit ea3a58d051
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 114 additions and 18 deletions

View File

@ -15,7 +15,14 @@ export default Component.extend(FilterModeMixin, {
// the `categories` property into this component
@discourseComputed("site.categoriesList")
categories(categoriesList) {
return categoriesList;
if (this.currentUser?.indirectly_muted_category_ids) {
return categoriesList.filter(
(category) =>
!this.currentUser.indirectly_muted_category_ids.includes(category.id)
);
} else {
return categoriesList;
}
},
@discourseComputed("category")

View File

@ -0,0 +1,44 @@
import { click } from "@ember/test-helpers";
import componentTest, {
setupRenderingTest,
} from "discourse/tests/helpers/component-test";
import { discourseModule, query } from "discourse/tests/helpers/qunit-helpers";
import hbs from "htmlbars-inline-precompile";
discourseModule("Integration | Component | d-navigation", function (hooks) {
setupRenderingTest(hooks);
componentTest("filters indirectly muted categories", {
template: hbs`
{{d-navigation
filterType="categories"
}}
`,
beforeEach() {
const categories = this.site.categoriesList
.filter((category) => !category.parent_category_id)
.slice(0, 4);
this.site.setProperties({
categoriesList: categories,
});
this.currentUser.set(
"indirectly_muted_category_ids",
categories.slice(0, 3).map((category) => category.id)
);
},
async test(assert) {
await click(".category-drop .select-kit-header-wrapper");
assert.strictEqual(
document.querySelectorAll(".category-row").length,
1,
"displays only categories that are not muted"
);
assert.strictEqual(
query(".category-row .badge-category span").textContent.trim(),
"dev"
);
},
});
});

View File

@ -235,26 +235,37 @@ class CategoryUser < ActiveRecord::Base
end
end
def self.indirectly_muted_category_ids(user)
query = Category.where.not(parent_category_id: nil)
def self.muted_category_ids_query(user, include_direct: false)
query = Category
query = query.where.not(parent_category_id: nil) if !include_direct
query = query
.joins("LEFT JOIN categories categories2 ON categories2.id = categories.parent_category_id")
.joins("LEFT JOIN category_users ON category_users.category_id = categories.id AND category_users.user_id = #{user.id}")
.joins("LEFT JOIN category_users category_users2 ON category_users2.category_id = categories2.id AND category_users2.user_id = #{user.id}")
.where("category_users.id IS NULL")
direct_category_muted_sql = "COALESCE(category_users.notification_level, #{CategoryUser.default_notification_level}) = #{CategoryUser.notification_levels[:muted]}"
parent_category_muted_sql =
"(category_users.id IS NULL AND COALESCE(category_users2.notification_level, #{CategoryUser.default_notification_level}) = #{notification_levels[:muted]})"
conditions = [parent_category_muted_sql]
conditions.push(direct_category_muted_sql) if include_direct
if SiteSetting.max_category_nesting === 3
query = query
.joins("LEFT JOIN categories categories3 ON categories3.id = categories2.parent_category_id")
.joins("LEFT JOIN category_users category_users3 ON category_users3.category_id = categories3.id AND category_users3.user_id = #{user.id}")
.where("
(category_users2.notification_level = #{notification_levels[:muted]})
OR
(category_users2.id IS NULL AND category_users3.notification_level = #{notification_levels[:muted]})
")
else
query = query.where("category_users2.notification_level = #{notification_levels[:muted]}")
grandparent_category_muted_sql = "(category_users.id IS NULL AND category_users2.id IS NULL AND COALESCE(category_users3.notification_level, #{CategoryUser.default_notification_level}) = #{notification_levels[:muted]})"
conditions.push(grandparent_category_muted_sql)
end
query.pluck("categories.id")
query.where(conditions.join(" OR "))
end
def self.muted_category_ids(user)
muted_category_ids_query(user, include_direct: true).pluck("categories.id")
end
def self.indirectly_muted_category_ids(user)
muted_category_ids_query(user).pluck("categories.id")
end
end

View File

@ -464,7 +464,6 @@ class TopicTrackingState
JOIN user_options AS uo ON uo.user_id = u.id
JOIN categories c ON c.id = topics.category_id
LEFT JOIN topic_users tu ON tu.topic_id = topics.id AND tu.user_id = u.id
LEFT JOIN category_users ON category_users.category_id = topics.category_id AND category_users.user_id = :user_id
#{skip_new ? "" : "LEFT JOIN dismissed_topic_users ON dismissed_topic_users.topic_id = topics.id AND dismissed_topic_users.user_id = :user_id"}
#{additional_join_sql}
WHERE u.id = :user_id AND
@ -478,7 +477,7 @@ class TopicTrackingState
NOT (
#{(skip_new && skip_unread) ? "" : "last_read_post_number IS NULL AND"}
(
COALESCE(category_users.notification_level, #{CategoryUser.default_notification_level}) = #{CategoryUser.notification_levels[:muted]}
topics.category_id IN (#{CategoryUser.muted_category_ids_query(user, include_direct: true).select("categories.id").to_sql})
AND tu.notification_level <= #{TopicUser.notification_levels[:regular]}
)
)

View File

@ -267,7 +267,7 @@ describe CategoryUser do
end
end
describe "#indirectly_muted_category_ids" do
describe ".muted_category_ids" do
context "max category nesting 2" do
fab!(:category1) { Fabricate(:category) }
fab!(:category2) { Fabricate(:category, parent_category: category1) }
@ -275,15 +275,19 @@ describe CategoryUser do
it "calculates muted categories based on parent category state" do
expect(CategoryUser.indirectly_muted_category_ids(user)).to eq([])
expect(CategoryUser.muted_category_ids(user)).to eq([])
category_user = CategoryUser.create!(user: user, category: category1, notification_level: CategoryUser.notification_levels[:muted])
expect(CategoryUser.indirectly_muted_category_ids(user)).to contain_exactly(category2.id, category3.id)
expect(CategoryUser.muted_category_ids(user)).to contain_exactly(category1.id, category2.id, category3.id)
CategoryUser.create!(user: user, category: category3, notification_level: CategoryUser.notification_levels[:muted])
expect(CategoryUser.indirectly_muted_category_ids(user)).to contain_exactly(category2.id)
expect(CategoryUser.muted_category_ids(user)).to contain_exactly(category1.id, category2.id, category3.id)
category_user.update(notification_level: CategoryUser.notification_levels[:regular])
category_user.update!(notification_level: CategoryUser.notification_levels[:regular])
expect(CategoryUser.indirectly_muted_category_ids(user)).to eq([])
expect(CategoryUser.muted_category_ids(user)).to contain_exactly(category3.id)
end
end
context "max category nesting 3" do
@ -302,16 +306,20 @@ describe CategoryUser do
CategoryUser.create!(user: user, category: category1, notification_level: CategoryUser.notification_levels[:muted])
expect(CategoryUser.indirectly_muted_category_ids(user)).to contain_exactly(category2.id, category3.id)
expect(CategoryUser.muted_category_ids(user)).to contain_exactly(category1.id, category2.id, category3.id)
category_user3 = CategoryUser.create!(user: user, category: category3, notification_level: CategoryUser.notification_levels[:muted])
expect(CategoryUser.indirectly_muted_category_ids(user)).to contain_exactly(category2.id)
expect(CategoryUser.muted_category_ids(user)).to contain_exactly(category1.id, category2.id, category3.id)
category_user3.destroy
category_user3.destroy!
category_user2 = CategoryUser.create!(user: user, category: category2, notification_level: CategoryUser.notification_levels[:muted])
expect(CategoryUser.indirectly_muted_category_ids(user)).to contain_exactly(category3.id)
expect(CategoryUser.muted_category_ids(user)).to contain_exactly(category1.id, category2.id, category3.id)
category_user2.update(notification_level: CategoryUser.notification_levels[:regular])
category_user2.update!(notification_level: CategoryUser.notification_levels[:regular])
expect(CategoryUser.indirectly_muted_category_ids(user)).to eq([])
expect(CategoryUser.muted_category_ids(user)).to contain_exactly(category1.id)
end
end
end

View File

@ -352,6 +352,33 @@ describe TopicTrackingState do
expect(report.length).to eq(1)
end
it "correctly handles indirectly muted categories" do
parent_category = Fabricate(:category)
sub_category = Fabricate(:category, parent_category_id: parent_category.id)
create_post(category: sub_category)
report = TopicTrackingState.report(user)
expect(report.length).to eq(1)
CategoryUser.create!(
user_id: user.id,
notification_level: CategoryUser.notification_levels[:muted],
category_id: parent_category.id
)
report = TopicTrackingState.report(user)
expect(report.length).to eq(0)
CategoryUser.create!(
user_id: user.id,
notification_level: CategoryUser.notification_levels[:regular],
category_id: sub_category.id
)
report = TopicTrackingState.report(user)
expect(report.length).to eq(1)
end
it "works when categories are default muted" do
SiteSetting.mute_all_categories_by_default = true