diff --git a/app/controllers/user_badges_controller.rb b/app/controllers/user_badges_controller.rb index abf2b9f054f..d87504d9a28 100644 --- a/app/controllers/user_badges_controller.rb +++ b/app/controllers/user_badges_controller.rb @@ -24,11 +24,19 @@ class UserBadgesController < ApplicationController user_badges = user_badges.offset(offset.to_i) end + user_badges_topic_ids = user_badges.map { |user_badge| user_badge.post&.topic_id }.compact + user_badges = UserBadges.new(user_badges: user_badges, username: params[:username], grant_count: grant_count) - render_serialized(user_badges, UserBadgesSerializer, root: :user_badge_info, include_long_description: true) + render_serialized( + user_badges, + UserBadgesSerializer, + root: :user_badge_info, + include_long_description: true, + allowed_user_badge_topic_ids: guardian.can_see_topic_ids(topic_ids: user_badges_topic_ids) + ) end def username @@ -46,9 +54,12 @@ class UserBadgesController < ApplicationController .includes(post: :topic) .includes(:granted_by) + user_badges_topic_ids = user_badges.map { |user_badge| user_badge.post&.topic_id }.compact + render_serialized( user_badges, DetailedUserBadgeSerializer, + allowed_user_badge_topic_ids: guardian.can_see_topic_ids(topic_ids: user_badges_topic_ids), root: :user_badges, ) end diff --git a/app/serializers/concerns/user_badge_post_and_topic_attributes_mixin.rb b/app/serializers/concerns/user_badge_post_and_topic_attributes_mixin.rb new file mode 100644 index 00000000000..3710dc5f23c --- /dev/null +++ b/app/serializers/concerns/user_badge_post_and_topic_attributes_mixin.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +# Post and topic attributes are only included when the `allowed_user_badge_topic_ids` option is provided. The caller of the +# serializer is responsible for ensuring that the topic ids in the options can be seen by the scope of the user by passing +# the result of `Guardian#can_see_topic_ids` to the `allowed_user_badge_topic_ids` option. +module UserBadgePostAndTopicAttributesMixin + private + + def include_post_attributes? + return false if !object.badge.show_posts || !object.post + return true if scope.is_admin? + + allowed_user_badge_topic_ids = options[:allowed_user_badge_topic_ids] + + return false if allowed_user_badge_topic_ids.blank? + + topic_id = object.post.topic_id + + return false if topic_id.blank? + + allowed_user_badge_topic_ids.include?(topic_id) + end + + def include_topic_attributes? + include_post_attributes? && object.post.topic + end +end diff --git a/app/serializers/detailed_user_badge_serializer.rb b/app/serializers/detailed_user_badge_serializer.rb index b924fffb26d..e31d8f8e8a7 100644 --- a/app/serializers/detailed_user_badge_serializer.rb +++ b/app/serializers/detailed_user_badge_serializer.rb @@ -1,27 +1,34 @@ # frozen_string_literal: true class DetailedUserBadgeSerializer < BasicUserBadgeSerializer + include UserBadgePostAndTopicAttributesMixin + has_one :granted_by, serializer: UserBadgeSerializer::UserSerializer attributes :post_number, :topic_id, :topic_title, :is_favorite, :can_favorite - def include_post_number? - object.post + def post_number + object.post.post_number end - alias :include_topic_id? :include_post_number? - alias :include_topic_title? :include_post_number? - - def post_number - object.post.post_number if object.post + def include_post_number? + include_post_attributes? end def topic_id - object.post.topic_id if object.post + object.post.topic_id + end + + def include_topic_id? + include_topic_attributes? end def topic_title - object.post.topic.title if object.post && object.post.topic + object.post.topic.title + end + + def include_topic_title? + include_topic_id? end def can_favorite diff --git a/app/serializers/user_badge_serializer.rb b/app/serializers/user_badge_serializer.rb index 6f904b01b51..4b2b536ef5a 100644 --- a/app/serializers/user_badge_serializer.rb +++ b/app/serializers/user_badge_serializer.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true class UserBadgeSerializer < ApplicationSerializer + include UserBadgePostAndTopicAttributesMixin class UserSerializer < BasicUserSerializer include UserPrimaryGroupMixin @@ -22,10 +23,9 @@ class UserBadgeSerializer < ApplicationSerializer end def include_post_id? - object.badge.show_posts && object.post_id && object.post + include_post_attributes? end - alias :include_topic? :include_post_id? alias :include_post_number? :include_post_id? def post_number @@ -35,4 +35,8 @@ class UserBadgeSerializer < ApplicationSerializer def topic object.post.topic end + + def include_topic? + include_topic_attributes? + end end diff --git a/spec/fabricators/user_badge_fabricator.rb b/spec/fabricators/user_badge_fabricator.rb new file mode 100644 index 00000000000..b1d8f259a48 --- /dev/null +++ b/spec/fabricators/user_badge_fabricator.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +Fabricator(:user_badge) do + user + badge + granted_at { Time.zone.now } + granted_by(fabricator: :admin) +end diff --git a/spec/requests/user_badges_controller_spec.rb b/spec/requests/user_badges_controller_spec.rb index 119c6de0fae..e4452d12081 100644 --- a/spec/requests/user_badges_controller_spec.rb +++ b/spec/requests/user_badges_controller_spec.rb @@ -28,7 +28,18 @@ RSpec.describe UserBadgesController do end describe '#index' do - let!(:user_badge) { UserBadge.create(badge: badge, user: user, granted_by: Discourse.system_user, granted_at: Time.now) } + fab!(:post) { Fabricate(:post) } + fab!(:private_message_post) { Fabricate(:private_message_post) } + let(:topic) { post.topic } + let(:private_message_topic) { private_message_post.topic } + fab!(:group) { Fabricate(:group) } + fab!(:private_category) { Fabricate(:private_category, group: group) } + fab!(:restricted_topic) { Fabricate(:topic, category: private_category) } + fab!(:restricted_post) { Fabricate(:post, topic: restricted_topic) } + fab!(:badge) { Fabricate(:badge, show_posts: true) } + fab!(:user_badge) { Fabricate(:user_badge, user: user, badge: badge, post: post) } + fab!(:user_badge_2) { Fabricate(:user_badge, badge: badge, post: private_message_post) } + fab!(:user_badge_3) { Fabricate(:user_badge, badge: badge, post: restricted_post) } it 'requires username or badge_id to be specified' do get "/user_badges.json" @@ -57,7 +68,7 @@ RSpec.describe UserBadgesController do expect(response.status).to eq(200) parsed = response.parsed_body - expect(parsed["user_badge_info"]["user_badges"].length).to eq(1) + expect(parsed["user_badge_info"]["user_badges"].length).to eq(3) end it 'includes counts when passed the aggregate argument' do @@ -70,6 +81,71 @@ RSpec.describe UserBadgesController do expect(parsed["user_badges"].first.has_key?('count')).to eq(true) end + context 'for post and topic attributes associated with user badge' do + it 'does not include the attributes for the private topic when user is anon' do + get "/user_badges.json", params: { badge_id: badge.id } + + expect(response.status).to eq(200) + + parsed = response.parsed_body + + expect(parsed["topics"].map { |t| t["id"] }).to contain_exactly(post.topic_id) + + parsed_user_badges = parsed["user_badge_info"]["user_badges"] + + expect(parsed_user_badges.map { |ub| ub["post_id"] }.compact).to contain_exactly(post.id) + expect(parsed_user_badges.map { |ub| ub["post_number"] }.compact).to contain_exactly(post.post_number) + end + + it 'does not include the attributes for topics which the current user cannot see' do + get "/user_badges.json", params: { badge_id: badge.id } + + expect(response.status).to eq(200) + + parsed = response.parsed_body + + expect(parsed["topics"].map { |t| t["id"] }).to contain_exactly(post.topic_id) + + parsed_user_badges = parsed["user_badge_info"]["user_badges"] + + expect(parsed_user_badges.map { |ub| ub["post_id"] }.compact).to contain_exactly(post.id) + expect(parsed_user_badges.map { |ub| ub["post_number"] }.compact).to contain_exactly(post.post_number) + end + + it 'includes the attributes for regular topic, private messages and restricted topics which the current user can see' do + group.add(user) + private_message_topic.allowed_users << user + + sign_in(user) + + get "/user_badges.json", params: { badge_id: badge.id } + + expect(response.status).to eq(200) + + parsed = response.parsed_body + + expect(parsed["topics"].map { |t| t["id"] }).to contain_exactly( + post.topic_id, + private_message_post.topic_id, + restricted_post.topic_id + ) + + parsed_user_badges = parsed["user_badge_info"]["user_badges"] + + expect(parsed_user_badges.map { |ub| ub["post_id"] }.compact).to contain_exactly( + post.id, + private_message_post.id, + restricted_post.id + ) + + expect(parsed_user_badges.map { |ub| ub["post_number"] }.compact).to contain_exactly( + post.post_number, + private_message_post.post_number, + restricted_post.post_number + ) + end + end + context 'with hidden profiles' do before do user.user_option.update_columns(hide_profile_and_presence: true) diff --git a/spec/serializers/detailed_user_badge_serializer_spec.rb b/spec/serializers/detailed_user_badge_serializer_spec.rb new file mode 100644 index 00000000000..6c4a713f10b --- /dev/null +++ b/spec/serializers/detailed_user_badge_serializer_spec.rb @@ -0,0 +1,89 @@ +# frozen_string_literal: true + +RSpec.describe DetailedUserBadgeSerializer do + describe '#topic_id and #topic_title attributes' do + fab!(:user) { Fabricate(:user) } + fab!(:admin) { Fabricate(:admin) } + fab!(:post) { Fabricate(:post) } + fab!(:badge) { Fabricate(:badge, show_posts: true) } + fab!(:user_badge) { Fabricate(:user_badge, badge: badge, post_id: post.id) } + let(:guardian) { Guardian.new(user_badge.user) } + + it 'does not include attributes in serialized object when badge has not been configured to show posts' do + badge.update!(show_posts: false) + + guardian = Guardian.new + + serialized = described_class.new(user_badge, + scope: guardian, + allowed_user_badge_topic_ids: guardian.can_see_topic_ids(topic_ids: [post.topic_id]), + root: false, + ).as_json + + expect(serialized[:topic_id]).to eq(nil) + expect(serialized[:topic_title]).to eq(nil) + end + + it 'does not include attributes in serialized object when user badge is not associated with a post' do + user_badge.update!(post_id: nil) + + serialized = described_class.new(user_badge, + scope: guardian, + allowed_user_badge_topic_ids: guardian.can_see_topic_ids(topic_ids: [post.topic_id]), + root: false + ).as_json + + expect(serialized[:topic_id]).to eq(nil) + expect(serialized[:topic_title]).to eq(nil) + end + + it 'does not include attributes in serialized object when user badge is not associated with a topic' do + post.topic.destroy! + + serialized = described_class.new(user_badge, + scope: guardian, + allowed_user_badge_topic_ids: guardian.can_see_topic_ids(topic_ids: [post.topic_id]), + root: false + ).as_json + + expect(serialized[:topic_id]).to eq(nil) + expect(serialized[:topic_title]).to eq(nil) + end + + it 'does not include attributes in serialized object when allowed_user_badge_topic_ids option is not provided' do + serialized = described_class.new(user_badge, scope: guardian, root: false).as_json + + expect(serialized[:topic_id]).to eq(nil) + expect(serialized[:topic_title]).to eq(nil) + end + + it 'does not included attributes in serialized object when topic id is not present in allowed_user_badge_topic_ids option' do + serialized = described_class.new(user_badge, + scope: guardian, + allowed_user_badge_topic_ids: guardian.can_see_topic_ids(topic_ids: [post.topic_id + 1]), + root: false + ).as_json + + expect(serialized[:topic_id]).to eq(nil) + expect(serialized[:topic_title]).to eq(nil) + end + + it 'includes attributes in serialized object for admin scope even if allowed_user_badge_topic_ids option is not provided' do + serialized = described_class.new(user_badge, scope: Guardian.new(admin), root: false).as_json + + expect(serialized[:topic_id]).to eq(post.topic_id) + expect(serialized[:topic_title]).to eq(post.topic.title) + end + + it 'includes attributes in serialized object when topic id is present in allowed_user_badge_topic_ids option' do + serialized = described_class.new(user_badge, + scope: guardian, + allowed_user_badge_topic_ids: guardian.can_see_topic_ids(topic_ids: [post.topic_id]), + root: false + ).as_json + + expect(serialized[:topic_id]).to eq(post.topic_id) + expect(serialized[:topic_title]).to eq(post.topic.title) + end + end +end diff --git a/spec/serializers/user_badge_serializer_spec.rb b/spec/serializers/user_badge_serializer_spec.rb new file mode 100644 index 00000000000..941c3445ef4 --- /dev/null +++ b/spec/serializers/user_badge_serializer_spec.rb @@ -0,0 +1,89 @@ +# frozen_string_literal: true + +RSpec.describe UserBadgeSerializer do + describe '#topic' do + fab!(:user) { Fabricate(:user) } + fab!(:admin) { Fabricate(:admin) } + fab!(:post) { Fabricate(:post) } + fab!(:badge) { Fabricate(:badge, show_posts: true) } + fab!(:user_badge) { Fabricate(:user_badge, badge: badge, post_id: post.id) } + let(:guardian) { Guardian.new(user_badge.user) } + + it 'is not included in serialized object when badge has not been configured to show posts' do + badge.update!(show_posts: false) + + guardian = Guardian.new + + serialized = described_class.new(user_badge, + scope: guardian, + allowed_user_badge_topic_ids: guardian.can_see_topic_ids(topic_ids: [post.topic_id]) + ).as_json + + expect(serialized[:topics]).to eq(nil) + end + + it 'is not included in serialized object when user badge is not associated with a post' do + user_badge.update!(post_id: nil) + + serialized = described_class.new(user_badge, + scope: guardian, + allowed_user_badge_topic_ids: guardian.can_see_topic_ids(topic_ids: [post.topic_id]) + ).as_json + + expect(serialized[:topics]).to eq(nil) + end + + it 'is not included in serialized object when user badge is not associated with a topic' do + post.topic.destroy! + + serialized = described_class.new(user_badge, + scope: guardian, + allowed_user_badge_topic_ids: guardian.can_see_topic_ids(topic_ids: [post.topic_id]) + ).as_json + + expect(serialized[:topics]).to eq(nil) + end + + it 'is not included in serialized object when allowed_user_badge_topic_ids option is not provided' do + serialized = described_class.new(user_badge, scope: guardian).as_json + + expect(serialized[:topics]).to eq(nil) + end + + it 'is not included in serialized object when topic id is not present in allowed_user_badge_topic_ids option' do + serialized = described_class.new(user_badge, + scope: guardian, + allowed_user_badge_topic_ids: guardian.can_see_topic_ids(topic_ids: [post.topic_id + 1]) + ).as_json + + expect(serialized[:topics]).to eq(nil) + end + + it 'is included in serialized object for admin scope even if allowed_user_badge_topic_ids option is not provided' do + serialized = described_class.new(user_badge, scope: Guardian.new(admin)).as_json + + serialized_topic = serialized[:topics][0] + + expect(serialized_topic[:id]).to eq(post.topic_id) + expect(serialized_topic[:title]).to eq(post.topic.title) + expect(serialized_topic[:fancy_title]).to eq(post.topic.fancy_title) + expect(serialized_topic[:slug]).to eq(post.topic.slug) + expect(serialized_topic[:posts_count]).to eq(post.topic.reload.posts_count) + end + + it 'is included in serialized object when topic id is present in allowed_user_badge_topic_ids option' do + serialized = described_class.new(user_badge, + scope: guardian, + allowed_user_badge_topic_ids: guardian.can_see_topic_ids(topic_ids: [post.topic_id]) + ).as_json + + serialized_topic = serialized[:topics][0] + + expect(serialized_topic[:id]).to eq(post.topic_id) + expect(serialized_topic[:title]).to eq(post.topic.title) + expect(serialized_topic[:fancy_title]).to eq(post.topic.fancy_title) + expect(serialized_topic[:slug]).to eq(post.topic.slug) + expect(serialized_topic[:posts_count]).to eq(post.topic.reload.posts_count) + end + end +end