Merge pull request #4369 from BrokenEagle/post-version-fixes

Various post version fixes

app/controllers/application_controller.rb

@@ -43,8 +43,8 @@ class ApplicationController < ActionController::Base
     super
   end
 
-  def set_version_comparison
-    params[:type] = %w[previous subsequent current].include?(params[:type]) ? params[:type] : "previous"
+  def set_version_comparison(default_type = "previous")
+    params[:type] = %w[previous subsequent current].include?(params[:type]) ? params[:type] : default_type
   end
 
   def model_name

app/controllers/post_versions_controller.rb

@@ -5,7 +5,7 @@ class PostVersionsController < ApplicationController
   respond_to :js, only: [:undo]
 
   def index
-    set_version_comparison
+    set_version_comparison("current")
     @post_versions = authorize PostVersion.paginated_search(params)
 
     if request.format.html?

app/helpers/post_versions_helper.rb

@@ -1,46 +1,51 @@
 module PostVersionsHelper
   def post_version_diff(post_version, type)
+    return "" if type == "previous" && post_version.version == 1
+
     other = post_version.send(type)
 
-    this_tags = post_version.tag_array
-    this_tags << "rating:#{post_version.rating}" if post_version.rating.present?
-    this_tags << "parent:#{post_version.parent_id}" if post_version.parent_id.present?
-    this_tags << "source:#{post_version.source}" if post_version.source.present?
+    added_tags = post_version.added_tags
+    added_tags << "rating:#{post_version_value(post_version.rating)}" if post_version.rating_changed
+    added_tags << "parent:#{post_version_value(post_version.parent_id)}" if post_version.parent_changed
+    added_tags << "source:#{post_version_value(post_version.source)}" if post_version.source_changed
 
-    other_tags = other.present? ? other.tag_array : []
-    if other.present?
-      other_tags << "rating:#{other.rating}" if other.rating.present?
-      other_tags << "parent:#{other.parent_id}" if other.parent_id.present?
-      other_tags << "source:#{other.source}" if other.source.present?
-    elsif type == "subsequent"
-      other_tags = this_tags
-    end
+    removed_tags = post_version.removed_tags
 
-    if type == "previous"
-      added_tags = this_tags - other_tags
-      removed_tags = other_tags - this_tags
+    if type == "previous" || other.nil?
+      obsolete_added_tags = []
+      obsolete_removed_tags = []
     else
-      added_tags = other_tags - this_tags
-      removed_tags = this_tags - other_tags
+      other_tags = other.tags.split
+      other_tags << "rating:#{post_version_value(other.rating)}"
+      other_tags << "parent:#{post_version_value(other.parent_id)}"
+      other_tags << "source:#{post_version_value(other.source)}"
+      obsolete_added_tags = added_tags - other_tags
+      obsolete_removed_tags = removed_tags & other_tags
     end
-    unchanged_tags = this_tags & other_tags
-
     html = '<span class="diff-list">'
 
     added_tags.each do |tag|
-      html << '<ins>+' + link_to(wordbreakify(tag), posts_path(:tags => tag)) + '</ins>'
-      html << " "
+      obsolete_class = (obsolete_added_tags.include?(tag) ? "diff-obsolete" : "");
+      html << %(<ins class="#{obsolete_class}">#{link_to(wordbreakify(tag), posts_path(:tags => tag))}</ins> )
     end
     removed_tags.each do |tag|
-      html << '<del>-' + link_to(wordbreakify(tag), posts_path(:tags => tag)) + '</del>'
-      html << " "
-    end
-    unchanged_tags.each do |tag|
-      html << '<span>' + link_to(wordbreakify(tag), posts_path(:tags => tag)) + '</span>'
-      html << " "
+      obsolete_class = (obsolete_removed_tags.include?(tag) ? "diff-obsolete" : "");
+      html << %(<del class="#{obsolete_class}">#{link_to(wordbreakify(tag), posts_path(:tags => tag))}</del> )
     end
 
     html << "</span>"
     html.html_safe
   end
+
+  def post_version_field(post_version, field)
+    value = post_version_value(post_version.send(field))
+    prefix = (field == :parent_id ? "parent" : field.to_s)
+    search = prefix + ":" + value.to_s
+    display = (field == :rating ? post_version.pretty_rating : value)
+    %(<b>#{field.to_s.titleize}:</b> #{link_to(display, posts_path(:tags => search))}).html_safe
+  end
+
+  def post_version_value(value)
+    return (value.present? ? value : "none")
+  end
 end

app/javascript/src/styles/base/040_colors.css

@@ -89,6 +89,8 @@
 
   --diff-list-added-color: green;
   --diff-list-removed-color: red;
+  --diff-list-obsolete-added-color: darkGreen;
+  --diff-list-obsolete-removed-color: darkRed;
 
   --wiki-page-versions-diff-del-background: #FCC;
   --wiki-page-versions-diff-ins-background: #CFC;
@@ -299,6 +301,8 @@ body[data-current-user-theme="dark"] {
 
   --diff-list-added-color: var(--green-1);
   --diff-list-removed-color: var(--red-1);
+  --diff-list-obsolete-added-color: var(--green-3);
+  --diff-list-obsolete-removed-color: var(--red-3);
 
   --dtext-blockquote-background: var(--grey-3);
   --dtext-blockquote-border: 1px solid var(--grey-4);

app/javascript/src/styles/specific/post_versions.scss

@@ -10,4 +10,19 @@ body.c-post-versions.a-index {
   .advanced-search-link {
     margin: 0 1em;
   }
+
+  ins.diff-obsolete a {
+    color: var(--diff-list-obsolete-added-color);
+    font-weight: bold;
+  }
+
+  del.diff-obsolete a {
+    color: var(--diff-list-obsolete-removed-color);
+    font-weight: bold;
+  }
+
+  td.tags-column,
+  td.edits-column {
+    width: 40%;
+  }
 }

app/models/post_version.rb

@@ -32,6 +32,13 @@ class PostVersion < ApplicationRecord
       end
     end
 
+    def tag_matches(string)
+      tag = string.split(/\S+/)[0]
+      return all if tag.nil?
+      tag = "*#{tag}*" unless tag =~ /\*/
+      where_ilike(:tags, tag)
+    end
+
     def search(params)
       q = super
       q = q.search_attributes(params, :updater_id, :post_id, :tags, :added_tags, :removed_tags, :rating, :rating_changed, :parent_id, :parent_changed, :source, :source_changed, :version)
@@ -40,10 +47,20 @@ class PostVersion < ApplicationRecord
         q = q.changed_tags_include_all(params[:changed_tags].scan(/[^[:space:]]+/))
       end
 
+      if params[:tag_matches]
+        q = q.tag_matches(params[:tag_matches])
+      end
+
       if params[:updater_name].present?
         q = q.where(updater_id: User.name_to_id(params[:updater_name]))
       end
 
+      if params[:is_new].to_s.truthy?
+        q = q.where(version: 1)
+      elsif params[:is_new].to_s.falsy?
+        q = q.where("version != 1")
+      end
+
       q.apply_default_order(params)
     end
   end
@@ -130,6 +147,19 @@ class PostVersion < ApplicationRecord
     }
   end
 
+  def pretty_rating
+    case rating
+    when "q"
+      "Questionable"
+
+    when "e"
+      "Explicit"
+
+    when "s"
+      "Safe"
+    end
+  end
+
   def changes
     delta = {
       :added_tags => added_tags,

app/views/post_versions/_listing.html.erb

@@ -3,34 +3,44 @@
     <%= PostPresenter.preview(@post_versions.first.post, show_deleted: true) %>
   <% end %>
 
-    <%= table_for @post_versions, {id: "post-versions-table", class: "striped autofit"} do |t| %>
+    <%= table_for @post_versions, id: "post-versions-table", class: "striped autofit", width: "100%" do |t| %>
       <% if policy(@post_versions).can_mass_undo? %>
-        <% t.column tag.label(tag.input type: :checkbox, id: "post-version-select-all-checkbox", class: "post-version-select-checkbox"), column: "post-version-select" do |post_version| %>
+        <% t.column tag.label(tag.input type: :checkbox, id: "post-version-select-all-checkbox", class: "post-version-select-checkbox"), column: "post-version-select", width: "1%" do |post_version| %>
           <input type="checkbox" class="post-version-select-checkbox" <%= "disabled" unless policy(post_version).undo? %>>
         <% end %>
       <% end %>
       <% if listing_type(:post_id) == :standard %>
-        <% t.column "Post" do |post_version| %>
+        <% t.column "Post", width: "1%" do |post_version| %>
           <%= PostPresenter.preview(post_version.post, show_deleted: true) %>
         <% end %>
       <% end %>
-      <% t.column "Version" do |post_version| %>
-        <%= link_to "#{post_version.post_id}.#{post_version.version}", post_versions_path(search: { post_id: post_version.post_id }, anchor: "post-version-#{post_version.id}") %>
+      <% t.column "Version", width: "1%" do |post_version| %>
+        <%= link_to "#{post_version.post_id}.#{post_version.version}", post_versions_path(search: { post_id: post_version.post_id }, type: params[:type], anchor: "post-version-#{post_version.id}") %>
       <% end %>
-      <% t.column "Tags", td: {class: "col-expand"} do |post_version| %>
+      <% t.column "Tags", td: {class: "col-expand"}, width: "40%" do |post_version| %>
+        <div>
+          <%= post_version_field(post_version, :rating) %>
+          <%= post_version_field(post_version, :parent_id) %>
+        </div>
+        <div><b>Tags:</b> <%= TagSetPresenter.new(post_version.tag_array).inline_tag_list_html %></div>
+        <div>
+          <%= post_version_field(post_version, :source) %>
+        </div>
+      <% end %>
+      <% t.column "Edits", td: {class: "col-expand"}, width: "40%" do |post_version| %>
         <%= post_version_diff(post_version, params[:type]) %>
       <% end %>
-      <% t.column "Changes" do |post_version| %>
+      <% t.column "Changes", width: "5%" do |post_version| %>
         <%= status_diff_html(post_version, params[:type]) %>
       <% end %>
-      <% t.column "Updated" do |post_version| %>
+      <% t.column "Updated", width: "5%" do |post_version| %>
         <%= link_to_user post_version.updater %>
         <%= link_to "»", post_versions_path(search: params[:search].merge({ updater_name: post_version.updater&.name })) %>
         <div>
           <%= compact_time(post_version.updated_at) %>
         </div>
       <% end %>
-      <% t.column do |post_version| %>
+      <% t.column column: "action", width: "5%" do |post_version| %>
         <% if policy(post_version).undo? %>
           <%= link_to "Undo", undo_post_version_path(post_version), method: :put, remote: true, class: "post-version-undo-link" %>
         <% end %>

app/views/post_versions/index.html.erb

@@ -15,7 +15,7 @@
       <%= f.input :removed_tags_include_all, label: "Removed Tags", input_html: { "data-autocomplete": "tag-query", value: params.dig(:search, :removed_tags_include_all) } %>
       <%= f.input :changed_tags, label: "Changed Tags", input_html: { "data-autocomplete": "tag-query", value: params.dig(:search, :changed_tags) } %>
       <%= f.submit "Search" %>
-      <%= link_to "Advanced", search_post_versions_path, class: "advanced-search-link" %>
+      <%= link_to "Advanced", search_post_versions_path(params.except(:controller, :action, :index, :commit, :type).permit!), class: "advanced-search-link" %>
     <% end %>
 
     <%= render "posts/partials/common/inline_blacklist" %>

app/views/post_versions/search.html.erb

@@ -3,19 +3,21 @@
     <h1>Search Changes</h1>
 
     <%= search_form_for(post_versions_path) do |f| %>
-      <%= f.input :updated_at, label: "Date" %>
-      <%= f.input :updater_name, label: "Updater", input_html: { "data-autocomplete": "user" } %>
-      <%= f.input :added_tags_include_all, label: "Added tags", input_html: { "data-autocomplete": "tag-query" } %>
-      <%= f.input :removed_tags_include_all, label: "Removed tags", input_html: { "data-autocomplete": "tag-query" } %>
-      <%= f.input :changed_tags, label: "Changed tags", input_html: { "data-autocomplete": "tag-query" } %>
-      <%= f.input :post_id %>
-      <%= f.input :parent_id %>
-      <%= f.input :rating %>
-      <%= f.input :source_ilike, label: "Source", hint: "Use * for wildcard" %>
-      <%= f.input :version %>
-      <%= f.input :rating_changed, as: :select %>
-      <%= f.input :parent_changed, as: :select %>
-      <%= f.input :source_changed, as: :select %>
+      <%= f.input :updated_at, label: "Date", input_html: { value: params.dig(:search, :updated_at) } %>
+      <%= f.input :updater_name, label: "Updater", input_html: { value: params.dig(:search, :updater_name), "data-autocomplete": "user" } %>
+      <%= f.input :added_tags_include_all, label: "Added tags", input_html: { value: params.dig(:search, :added_tags_include_all), "data-autocomplete": "tag-query" } %>
+      <%= f.input :removed_tags_include_all, label: "Removed tags", input_html: { value: params.dig(:search, :removed_tags_include_all), "data-autocomplete": "tag-query" } %>
+      <%= f.input :changed_tags, label: "Changed tags", input_html: { value: params.dig(:search, :changed_tags), "data-autocomplete": "tag-query" } %>
+      <%= f.input :post_id, input_html: { value: params.dig(:search, :post_id) } %>
+      <%= f.input :parent_id, input_html: { value: params.dig(:search, :parent_id) } %>
+      <%= f.input :rating, input_html: { value: params.dig(:search, :rating) } %>
+      <%= f.input :source_ilike, label: "Source", input_html: { value: params.dig(:search, :source_ilike) }, hint: "Use * for wildcard" %>
+      <%= f.input :tag_matches, input_html: { value: params.dig(:search, :tag_matches) }, hint: "Single tag, use * for wildcard" %>
+      <%= f.input :version, input_html: { value: params.dig(:search, :version) } %>
+      <%= f.input :is_new, label: "Include uploads?", collection: [["Yes", nil], ["No", false], ["Only", true]], include_blank: false, selected: params.dig(:search, :is_new), hint: "I.e. the 1st version of a post" %>
+      <%= f.input :rating_changed, as: :select, include_blank: true, selected: params.dig(:search, :rating_changed) %>
+      <%= f.input :parent_changed, as: :select, include_blank: true, selected: params.dig(:search, :parent_changed) %>
+      <%= f.input :source_changed, as: :select, include_blank: true, selected: params.dig(:search, :source_changed) %>
       <%= f.submit "Search" %>
     <% end %>
   </div>