From 1cfb5129fa5578ade5bd136056872ad899bb7d1c Mon Sep 17 00:00:00 2001 From: Emi Matchu Date: Sun, 22 Jun 2025 11:35:27 -0700 Subject: [PATCH] Hide shadowbanned users from trade lists for most viewers Users will still see their own trades in the list and in the counts, and support staff will still see them too! --- app/controllers/closet_hangers_controller.rb | 8 ++------ app/controllers/item_trades_controller.rb | 3 ++- app/controllers/items_controller.rb | 3 ++- app/models/closet_hanger.rb | 13 ++++++++----- app/models/user.rb | 11 +++++++++++ app/views/item_trades/index.html.haml | 5 ++++- 6 files changed, 29 insertions(+), 14 deletions(-) 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