From 11a8c2877b81a91ed0b97664d59bea47272a1923 Mon Sep 17 00:00:00 2001
From: evazion <noizave@gmail.com>
Date: Sat, 2 Jan 2021 12:57:18 -0600
Subject: [PATCH] favorites: refactor favlist order on post page.

On the posts show page, in the favorites list, show favorites according
to the order they were added to the favorites table, rather than the
order they were added to the posts's fav_string.

On most posts these should be the same, but on old posts they may be
slightly different. The IDs of the first few hundred thousand favorites
don't appear to be in chronological order. Probably the original
favorite IDs were lost and recreated by a database move at some point in
Danbooru's history. The fav_string is also inconsistent with the
favorites table in some places (one contains favorites that aren't
contained by the other), which also throws off the order.

Partially addresses #4562 by eliminating one place where we depended on
the fav_string.
---
 app/helpers/posts_helper.rb                          |  4 ----
 app/models/post.rb                                   | 12 +++++-------
 app/views/favorites/_update.js.erb                   |  2 +-
 .../posts/partials/show/_favorite_list.html.erb      |  2 ++
 app/views/posts/partials/show/_information.html.erb  |  4 +++-
 5 files changed, 11 insertions(+), 13 deletions(-)
 create mode 100644 app/views/posts/partials/show/_favorite_list.html.erb

diff --git a/app/helpers/posts_helper.rb b/app/helpers/posts_helper.rb
index 378ad5a71..a521b20e8 100644
--- a/app/helpers/posts_helper.rb
+++ b/app/helpers/posts_helper.rb
@@ -48,10 +48,6 @@ module PostsHelper
     end
   end
 
-  def post_favlist(post)
-    post.favorited_users.reverse_each.map {|user| link_to_user(user)}.join(", ").html_safe
-  end
-
   def is_pool_selected?(pool)
     return false if params.key?(:q)
     return false if params.key?(:favgroup_id)
diff --git a/app/models/post.rb b/app/models/post.rb
index e89d79e34..4e3300515 100644
--- a/app/models/post.rb
+++ b/app/models/post.rb
@@ -56,6 +56,7 @@ class Post < ApplicationRecord
   has_many :approvals, :class_name => "PostApproval", :dependent => :destroy
   has_many :disapprovals, :class_name => "PostDisapproval", :dependent => :destroy
   has_many :favorites
+  has_many :favorited_users, through: :favorites, source: :user
   has_many :replacements, class_name: "PostReplacement", :dependent => :destroy
 
   attr_accessor :old_tag_string, :old_parent_id, :old_source, :old_rating, :has_constraints, :disable_versioning, :view_count
@@ -802,14 +803,11 @@ class Post < ApplicationRecord
       false
     end
 
-    # users who favorited this post, ordered by users who favorited it first
-    def favorited_users
-      favorited_user_ids = fav_string.scan(/\d+/).map(&:to_i)
-      visible_users = User.find(favorited_user_ids).select do |user|
-        Pundit.policy!([CurrentUser.user, nil], user).can_see_favorites?
+    # Users who publicly favorited this post, ordered by time of favorite.
+    def visible_favorited_users(viewer)
+      favorited_users.order("favorites.id DESC").select do |fav_user|
+        Pundit.policy!([viewer, nil], fav_user).can_see_favorites?
       end
-      ordered_users = visible_users.index_by(&:id).slice(*favorited_user_ids).values
-      ordered_users
     end
 
     def favorite_groups
diff --git a/app/views/favorites/_update.js.erb b/app/views/favorites/_update.js.erb
index 73b71ec61..0c2ece4aa 100644
--- a/app/views/favorites/_update.js.erb
+++ b/app/views/favorites/_update.js.erb
@@ -14,7 +14,7 @@ $(".fav-buttons").toggleClass("fav-buttons-false").toggleClass("fav-buttons-true
 
 <% if policy(@post).can_view_favlist? %>
   var fav_count = <%= @post.fav_count %>;
-  $("#favlist").html("<%= j post_favlist(@post) %>");
+  $("#favlist").html("<%= j render "posts/partials/show/favorite_list", post: @post %>");
 
   if (fav_count === 0) {
     $("#show-favlist-link, #hide-favlist-link, #favlist").hide();
diff --git a/app/views/posts/partials/show/_favorite_list.html.erb b/app/views/posts/partials/show/_favorite_list.html.erb
new file mode 100644
index 000000000..863a24a7a
--- /dev/null
+++ b/app/views/posts/partials/show/_favorite_list.html.erb
@@ -0,0 +1,2 @@
+<%# post %>
+<%= safe_join(post.visible_favorited_users(CurrentUser.user).map { |user| link_to_user(user) }, ", ") %>
diff --git a/app/views/posts/partials/show/_information.html.erb b/app/views/posts/partials/show/_information.html.erb
index dcfc2b11c..51ce633b2 100644
--- a/app/views/posts/partials/show/_information.html.erb
+++ b/app/views/posts/partials/show/_information.html.erb
@@ -37,7 +37,9 @@
     <% if policy(post).can_view_favlist? %>
     <%= link_to "Show »", "#", id: "show-favlist-link", style: ("display: none;" if post.fav_count == 0) %>
     <%= link_to "« Hide", "#", id: "hide-favlist-link", style: "display: none;" %>
-    <div id="favlist" style="display: none;"><%= post_favlist(post) %></div>
+    <div id="favlist" style="display: none;">
+      <%= render "posts/partials/show/favorite_list", post: post %>
+    </div>
   <% end %></li>
   <li id="post-info-status">
     Status: