diff --git a/app/models/category.rb b/app/models/category.rb index 6692d2d1fdd..93ee75536e2 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -91,6 +91,7 @@ class Category < ActiveRecord::Base after_commit :trigger_category_created_event, on: :create after_commit :trigger_category_updated_event, on: :update after_commit :trigger_category_destroyed_event, on: :destroy + after_commit :clear_site_cache after_save_commit :index_search @@ -957,6 +958,10 @@ class Category < ActiveRecord::Base result.map { |row| [row.group_id, row.permission_type] } end + + def clear_site_cache + Site.clear_cache + end end # == Schema Information diff --git a/app/models/category_tag.rb b/app/models/category_tag.rb index 1e21409c31d..e9cba7c189e 100644 --- a/app/models/category_tag.rb +++ b/app/models/category_tag.rb @@ -3,6 +3,10 @@ class CategoryTag < ActiveRecord::Base belongs_to :category belongs_to :tag + + after_commit do + Site.clear_cache + end end # == Schema Information diff --git a/app/models/category_tag_group.rb b/app/models/category_tag_group.rb index 06e64ad65ff..ea27bc50c15 100644 --- a/app/models/category_tag_group.rb +++ b/app/models/category_tag_group.rb @@ -3,6 +3,10 @@ class CategoryTagGroup < ActiveRecord::Base belongs_to :category belongs_to :tag_group + + after_commit do + Site.clear_cache + end end # == Schema Information diff --git a/app/models/site.rb b/app/models/site.rb index f8c131ef1d0..cf59a1f156a 100644 --- a/app/models/site.rb +++ b/app/models/site.rb @@ -28,16 +28,42 @@ class Site UserField.order(:position).all end - def categories - @categories ||= begin + CATEGORIES_CACHE_KEY = "site_categories" + + def self.clear_cache + Discourse.cache.delete(CATEGORIES_CACHE_KEY) + end + + def self.all_categories_cache + # Categories do not change often so there is no need for us to run the + # same query and spend time creating ActiveRecord objects for every requests. + # + # Do note that any new association added to the eager loading needs a + # corresponding ActiveRecord callback to clear the categories cache. + Discourse.cache.fetch(CATEGORIES_CACHE_KEY, expires_in: 30.minutes) do categories = Category - .includes(:uploaded_logo, :uploaded_background, :tags, :tag_groups) - .secured(@guardian) + .includes(:uploaded_logo, :uploaded_background, :tags, :tag_groups, :required_tag_group) .joins('LEFT JOIN topics t on t.id = categories.topic_id') .select('categories.*, t.slug topic_slug') .order(:position) + .to_a - categories = categories.to_a + ActiveModel::ArraySerializer.new( + categories, + each_serializer: SiteCategorySerializer + ).as_json + end + end + + def categories + @categories ||= begin + categories = [] + + self.class.all_categories_cache.each do |category| + if @guardian.can_see_serialized_category?(category_id: category[:id], read_restricted: category[:read_restricted]) + categories << OpenStruct.new(category) + end + end with_children = Set.new categories.each do |c| diff --git a/app/serializers/site_serializer.rb b/app/serializers/site_serializer.rb index 40da58e355e..c7d8d5d66c2 100644 --- a/app/serializers/site_serializer.rb +++ b/app/serializers/site_serializer.rb @@ -30,10 +30,10 @@ class SiteSerializer < ApplicationSerializer :shared_drafts_category_id, :custom_emoji_translation, :watched_words_replace, - :watched_words_link + :watched_words_link, + :categories ) - has_many :categories, serializer: SiteCategorySerializer, embed: :objects has_many :archetypes, embed: :objects, serializer: ArchetypeSerializer has_many :user_fields, embed: :objects, serializer: UserFieldSerializer has_many :auth_providers, embed: :objects, serializer: AuthProviderSerializer @@ -190,6 +190,10 @@ class SiteSerializer < ApplicationSerializer WordWatcher.word_matcher_regexps(:link) end + def categories + object.categories.map { |c| c.to_h } + end + private def ordered_flags(flags) diff --git a/lib/guardian/category_guardian.rb b/lib/guardian/category_guardian.rb index 4e9050f29a3..ef8441071b6 100644 --- a/lib/guardian/category_guardian.rb +++ b/lib/guardian/category_guardian.rb @@ -46,6 +46,14 @@ module CategoryGuardian nil end + def can_see_serialized_category?(category_id:, read_restricted: true) + # Guard to ensure only a boolean is passed in + read_restricted = true unless !!read_restricted == read_restricted + + return true if !read_restricted + secure_category_ids.include?(category_id) + end + def can_see_category?(category) return false unless category return true if is_admin? diff --git a/spec/models/site_spec.rb b/spec/models/site_spec.rb index 78b3faec630..637c812de85 100644 --- a/spec/models/site_spec.rb +++ b/spec/models/site_spec.rb @@ -58,30 +58,65 @@ describe Site do expect(Site.new(guardian).categories.last.notification_level).to eq(1) end - it "omits categories users can not write to from the category list" do - category = Fabricate(:category) - user = Fabricate(:user) + describe '#categories' do + fab!(:category) { Fabricate(:category) } + fab!(:user) { Fabricate(:user) } + fab!(:guardian) { Guardian.new(user) } - expect(Site.new(Guardian.new(user)).categories.count).to eq(2) + after do + Site.clear_cache + end - category.set_permissions(everyone: :create_post) - category.save + it "omits read restricted categories" do + expect(Site.new(guardian).categories.map(&:id)).to contain_exactly( + SiteSetting.uncategorized_category_id, category.id + ) - guardian = Guardian.new(user) + category.update!(read_restricted: true) - expect(Site.new(guardian) - .categories - .keep_if { |c| c.name == category.name } - .first - .permission) - .not_to eq(CategoryGroup.permission_types[:full]) + expect(Site.new(guardian).categories.map(&:id)).to contain_exactly( + SiteSetting.uncategorized_category_id + ) + end - # If a parent category is not visible, the child categories should not be returned - category.set_permissions(staff: :full) - category.save + it "includes categories that a user's group can see" do + group = Fabricate(:group) + category.update!(read_restricted: true) + category.groups << group - sub_category = Fabricate(:category, parent_category_id: category.id) - expect(Site.new(guardian).categories).not_to include(sub_category) + expect(Site.new(guardian).categories.map(&:id)).to contain_exactly( + SiteSetting.uncategorized_category_id + ) + + group.add(user) + + expect(Site.new(Guardian.new(user)).categories.map(&:id)).to contain_exactly( + SiteSetting.uncategorized_category_id, category.id + ) + end + + it "omits categories users can not write to from the category list" do + expect(Site.new(guardian).categories.count).to eq(2) + + category.set_permissions(everyone: :create_post) + category.save! + + guardian = Guardian.new(user) + + expect(Site.new(guardian) + .categories + .keep_if { |c| c.name == category.name } + .first + .permission) + .not_to eq(CategoryGroup.permission_types[:full]) + + # If a parent category is not visible, the child categories should not be returned + category.set_permissions(staff: :full) + category.save! + + sub_category = Fabricate(:category, parent_category_id: category.id) + expect(Site.new(guardian).categories).not_to include(sub_category) + end end it "omits groups user can not see" do diff --git a/spec/serializers/site_serializer_spec.rb b/spec/serializers/site_serializer_spec.rb index 1a53cfd11ff..5bf58564c0c 100644 --- a/spec/serializers/site_serializer_spec.rb +++ b/spec/serializers/site_serializer_spec.rb @@ -10,13 +10,34 @@ describe SiteSerializer do category.custom_fields["enable_marketplace"] = true category.save_custom_fields - data = MultiJson.dump(described_class.new(Site.new(guardian), scope: guardian, root: false)) - expect(data).not_to include("enable_marketplace") + serialized = described_class.new(Site.new(guardian), scope: guardian, root: false).as_json + c1 = serialized[:categories].find { |c| c[:id] == category.id } + + expect(c1[:preloaded_custom_fields]).to eq(nil) Site.preloaded_category_custom_fields << "enable_marketplace" - data = MultiJson.dump(described_class.new(Site.new(guardian), scope: guardian, root: false)) - expect(data).to include("enable_marketplace") + serialized = described_class.new(Site.new(guardian), scope: guardian, root: false).as_json + c1 = serialized[:categories].find { |c| c[:id] == category.id } + + expect(c1[:preloaded_custom_fields]["enable_marketplace"]).to eq("t") + end + + it "includes category tags" do + tag = Fabricate(:tag) + tag_group = Fabricate(:tag_group) + tag_group_2 = Fabricate(:tag_group) + + category.tags << tag + category.tag_groups << tag_group + category.update!(required_tag_group: tag_group_2) + + serialized = described_class.new(Site.new(guardian), scope: guardian, root: false).as_json + c1 = serialized[:categories].find { |c| c[:id] == category.id } + + expect(c1[:allowed_tags]).to contain_exactly(tag.name) + expect(c1[:allowed_tag_groups]).to contain_exactly(tag_group.name) + expect(c1[:required_tag_group_name]).to eq(tag_group_2.name) end it "returns correct notification level for categories" do