From 6aa0adb7380da382c1fb4144f7002abf83358495 Mon Sep 17 00:00:00 2001 From: BrokenEagle Date: Sat, 28 Mar 2020 03:23:13 +0000 Subject: [PATCH 1/3] Make the post versions more like the old format - "Current" is now most like the old format -- It is therefore now the default for post versions - Only show the actual edits in their own column - Show the current state at that version in another column - On the "previous" view, don't double-show full list of tags for the first post versions, so leave edits blank --- app/controllers/application_controller.rb | 4 +- app/controllers/post_versions_controller.rb | 2 +- app/helpers/post_versions_helper.rb | 59 ++++++++++--------- app/javascript/src/styles/base/040_colors.css | 4 ++ .../src/styles/specific/post_versions.scss | 15 +++++ app/models/post_version.rb | 13 ++++ app/views/post_versions/_listing.html.erb | 28 ++++++--- 7 files changed, 86 insertions(+), 39 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index b5f8ebdf5..75985eac6 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -42,8 +42,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 diff --git a/app/controllers/post_versions_controller.rb b/app/controllers/post_versions_controller.rb index 773fd2ff9..5a44fafe7 100644 --- a/app/controllers/post_versions_controller.rb +++ b/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? diff --git a/app/helpers/post_versions_helper.rb b/app/helpers/post_versions_helper.rb index a4d8f1144..ddc6509b8 100644 --- a/app/helpers/post_versions_helper.rb +++ b/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 = '' added_tags.each do |tag| - html << '+' + link_to(wordbreakify(tag), posts_path(:tags => tag)) + '' - html << " " + obsolete_class = (obsolete_added_tags.include?(tag) ? "diff-obsolete" : ""); + html << %(#{link_to(wordbreakify(tag), posts_path(:tags => tag))} ) end removed_tags.each do |tag| - html << '-' + link_to(wordbreakify(tag), posts_path(:tags => tag)) + '' - html << " " - end - unchanged_tags.each do |tag| - html << '' + link_to(wordbreakify(tag), posts_path(:tags => tag)) + '' - html << " " + obsolete_class = (obsolete_removed_tags.include?(tag) ? "diff-obsolete" : ""); + html << %(#{link_to(wordbreakify(tag), posts_path(:tags => tag))} ) end html << "" 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) + %(#{field.to_s.titleize}: #{link_to(display, posts_path(:tags => search))}).html_safe + end + + def post_version_value(value) + return (value.present? ? value : "none") + end end diff --git a/app/javascript/src/styles/base/040_colors.css b/app/javascript/src/styles/base/040_colors.css index beef5495a..1b7c51541 100644 --- a/app/javascript/src/styles/base/040_colors.css +++ b/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); diff --git a/app/javascript/src/styles/specific/post_versions.scss b/app/javascript/src/styles/specific/post_versions.scss index 7c8bf1f9b..bff0195b7 100644 --- a/app/javascript/src/styles/specific/post_versions.scss +++ b/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%; + } } diff --git a/app/models/post_version.rb b/app/models/post_version.rb index d45ea0e64..13fae86bf 100644 --- a/app/models/post_version.rb +++ b/app/models/post_version.rb @@ -130,6 +130,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, diff --git a/app/views/post_versions/_listing.html.erb b/app/views/post_versions/_listing.html.erb index 04eb02531..ffcf92c6d 100644 --- a/app/views/post_versions/_listing.html.erb +++ b/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| %> > <% 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| %> +
+ <%= post_version_field(post_version, :rating) %> + <%= post_version_field(post_version, :parent_id) %> +
+
Tags: <%= TagSetPresenter.new(post_version.tag_array).inline_tag_list_html %>
+
+ <%= post_version_field(post_version, :source) %> +
+ <% 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 })) %>
<%= compact_time(post_version.updated_at) %>
<% 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 %> From 4d9eae0038098f3680ab4f76e53ac8b632d577f5 Mon Sep 17 00:00:00 2001 From: BrokenEagle Date: Sat, 28 Mar 2020 04:29:12 +0000 Subject: [PATCH 2/3] Add additional advanced search operators - Tag matches allows a user to search for a single tag -- Since testing for multiple tags would require converting the "tags" string to an array which would most likely fail even for single tags - Is new for quick searching of uploads or not uploads --- app/models/post_version.rb | 17 +++++++++++++++++ app/views/post_versions/search.html.erb | 2 ++ 2 files changed, 19 insertions(+) diff --git a/app/models/post_version.rb b/app/models/post_version.rb index 13fae86bf..701742d67 100644 --- a/app/models/post_version.rb +++ b/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 diff --git a/app/views/post_versions/search.html.erb b/app/views/post_versions/search.html.erb index 0413edc9b..926ed4ca9 100644 --- a/app/views/post_versions/search.html.erb +++ b/app/views/post_versions/search.html.erb @@ -12,7 +12,9 @@ <%= f.input :parent_id %> <%= f.input :rating %> <%= f.input :source_ilike, label: "Source", hint: "Use * for wildcard" %> + <%= f.input :tag_matches, hint: "Use * for wildcard" %> <%= f.input :version %> + <%= f.input :is_new, label: "Include uploads?", collection: [["Yes", nil], ["No", false], ["Only", true]], include_blank: false, hint: "I.e. the 1st versions of posts" %> <%= f.input :rating_changed, as: :select %> <%= f.input :parent_changed, as: :select %> <%= f.input :source_changed, as: :select %> From 472c00e27bebb89373c7d99f56bd0298edd37e54 Mon Sep 17 00:00:00 2001 From: BrokenEagle Date: Sat, 28 Mar 2020 05:19:50 +0000 Subject: [PATCH 3/3] Retain search parameters when navigating from results to advanced search --- app/views/post_versions/index.html.erb | 2 +- app/views/post_versions/search.html.erb | 30 ++++++++++++------------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/app/views/post_versions/index.html.erb b/app/views/post_versions/index.html.erb index 4e864b748..246ecaac4 100644 --- a/app/views/post_versions/index.html.erb +++ b/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" %> diff --git a/app/views/post_versions/search.html.erb b/app/views/post_versions/search.html.erb index 926ed4ca9..5f862cb68 100644 --- a/app/views/post_versions/search.html.erb +++ b/app/views/post_versions/search.html.erb @@ -3,21 +3,21 @@

Search Changes

<%= 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 :tag_matches, hint: "Use * for wildcard" %> - <%= f.input :version %> - <%= f.input :is_new, label: "Include uploads?", collection: [["Yes", nil], ["No", false], ["Only", true]], include_blank: false, hint: "I.e. the 1st versions of posts" %> - <%= 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 %>