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/assets/javascripts/closet_hangers/index.js

@@ -754,6 +754,11 @@
 						contactField.val(connection.id);
 						submitContactForm();
 					},
+					error: function (xhr) {
+						var data = JSON.parse(xhr.responseText);
+						var fullMessage = data.full_error_messages.join("\n");
+						alert("Oops, we couldn't save this username!\n\n" + fullMessage);
+					},
 				});
 			}
 		} else {

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
 

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?

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
 

app/controllers/neopets_connections_controller.rb

@@ -5,7 +5,10 @@ class NeopetsConnectionsController < ApplicationController
     if connection.save
       render json: connection
     else
-      render json: {error: 'failure'}, status: :internal_server_error
+      render json: {
+        errors: connection.errors,
+        full_error_messages: connection.errors.map(&:full_message)
+      }, status: :bad_request
     end
   end
 

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

app/models/neopets_connection.rb

@@ -1,5 +1,6 @@
 class NeopetsConnection < ApplicationRecord
   belongs_to :user
 
-  validates :neopets_username, uniqueness: {scope: :user_id}
+  validates :neopets_username, uniqueness: {scope: :user_id},
+    format: { without: /@/, message: 'must not be an email address, for user safety' }
 end

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

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