diff --git a/app/controllers/closet_hangers_controller.rb b/app/controllers/closet_hangers_controller.rb index 2e2455b7..464b51bf 100644 --- a/app/controllers/closet_hangers_controller.rb +++ b/app/controllers/closet_hangers_controller.rb @@ -218,12 +218,8 @@ class ClosetHangersController < ApplicationController def enforce_shadowban # If this user is shadowbanned, and this *doesn't* seem to be a request # from that user, render the 404 page. - if @user.shadowbanned? - can_see = support_staff? || - @user.likely_is?(current_user, request.remote_ip) - if !can_see - render file: "public/404.html", layout: false, status: :not_found - end + if !@user.visible_to?(current_user, request.remote_ip) + render file: "public/404.html", layout: false, status: :not_found end end diff --git a/app/controllers/item_trades_controller.rb b/app/controllers/item_trades_controller.rb index a96f662d..8a31cda4 100644 --- a/app/controllers/item_trades_controller.rb +++ b/app/controllers/item_trades_controller.rb @@ -4,7 +4,8 @@ class ItemTradesController < ApplicationController @type = type_from_params @item_trades = @item.closet_hangers.trading.includes(:user, :list). - user_is_active.order('users.last_trade_activity_at DESC').to_trades + user_is_active.order('users.last_trade_activity_at DESC'). + to_trades(current_user, request.remote_ip) @trades = @item_trades[@type] if user_signed_in? diff --git a/app/controllers/items_controller.rb b/app/controllers/items_controller.rb index 582ce5e9..80ee4109 100644 --- a/app/controllers/items_controller.rb +++ b/app/controllers/items_controller.rb @@ -80,7 +80,8 @@ class ItemsController < ApplicationController respond_to do |format| format.html do - @trades = @item.closet_hangers.trading.user_is_active.to_trades + @trades = @item.closet_hangers.trading.user_is_active. + to_trades(current_user, request.remote_ip) @contributors_with_counts = @item.contributors_with_counts diff --git a/app/models/closet_hanger.rb b/app/models/closet_hanger.rb index 7d3675a8..769e18a1 100644 --- a/app/models/closet_hanger.rb +++ b/app/models/closet_hanger.rb @@ -156,7 +156,7 @@ class ClosetHanger < ApplicationRecord # # We don't preload anything here - if you want user names or list names, you # should `includes` them in the hanger scope first, to avoid extra queries! - def self.to_trades + def self.to_trades(current_user, remote_ip) # Let's ensure that the `trading` filter is applied, to avoid data leaks. # (I still recommend doing it at the call site too for clarity, though!) all_trading_hangers = trading.to_a @@ -164,17 +164,20 @@ class ClosetHanger < ApplicationRecord owned_hangers = all_trading_hangers.filter(&:owned?) wanted_hangers = all_trading_hangers.filter(&:wanted?) - # Group first into offering vs seeking, then by user. + # Group first into offering vs seeking, then by user. Only include trades + # visible to the current user. offering, seeking = [owned_hangers, wanted_hangers].map do |hangers| - hangers.group_by(&:user_id).map do |user_id, user_hangers| - Trade.new(user_id, user_hangers) - end + hangers.group_by(&:user_id). + map { |user_id, user_hangers| Trade.new(user_id, user_hangers) }. + filter { |trade| trade.visible_to?(current_user, remote_ip) } end {offering: offering, seeking: seeking} end Trade = Struct.new('Trade', :user_id, :hangers) do + delegate :visible_to?, to: :user + def user # Take advantage of `includes(:user)` on the hangers, if applied. hangers.first.user diff --git a/app/models/user.rb b/app/models/user.rb index 2a2cc634..a70a2d81 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -198,6 +198,17 @@ class User < ApplicationRecord touch(:last_trade_activity_at) end + def visible_to?(current_user, remote_ip) + # Everyone is visible to support staff. + return true if current_user&.support_staff? + + # Shadowbanned users are only visible to themselves. + return false if shadowbanned? && !likely_is?(current_user, remote_ip) + + # Other than that, users are visible to everyone by default. + return true + end + def self.points_required_to_pass_top_contributor(offset) user = User.top_contributors.select(:points).limit(1).offset(offset).first user ? user.points : 0 diff --git a/app/views/item_trades/index.html.haml b/app/views/item_trades/index.html.haml index ae0cea28..4a5c2cf9 100644 --- a/app/views/item_trades/index.html.haml +++ b/app/views/item_trades/index.html.haml @@ -24,7 +24,10 @@ 'data-is-same-as-prev': same_vague_trade_timestamp?(trade, prev_trade) } = vague_trade_timestamp trade - %td= trade.user.name + %td + = trade.user.name + - if support_staff? && trade.user.shadowbanned? + %abbr{title: "Shadowbanned (Hidden from most viewers; you can see because you're Support staff"} 🕶️ SBan %td - if trade.lists.present? %ul.trade-list-names