From d2021256f0ffbde994cbce5eb409966b3d6dcd78 Mon Sep 17 00:00:00 2001 From: BrokenEagle Date: Sat, 8 Feb 2020 04:15:33 +0000 Subject: [PATCH 01/10] Fix instances where a carriage return is replaced with another - It was causing two
elements to be inserted - Now the delete and insert paragraph marks are located next to each other --- app/logical/diff_builder.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/logical/diff_builder.rb b/app/logical/diff_builder.rb index e3d7192ee..1d623261f 100644 --- a/app/logical/diff_builder.rb +++ b/app/logical/diff_builder.rb @@ -20,6 +20,14 @@ class DiffBuilder output.each { |q| q.replace(escape_html[q]) } diffs.reverse_each do |hunk| + old_cr = hunk[0].try(:old_element) + new_cr = hunk[1].try(:new_element) + if old_cr && new_cr && old_cr.match?(/^\r?\n$/) && new_cr.match?(/^\r?\n$/) + hunk_position = hunk[0].old_position + output[hunk_position] = '
' + next + end + newchange = hunk.max {|a, b| a.old_position <=> b.old_position} newstart = newchange.old_position oldstart = hunk.min {|a, b| a.old_position <=> b.old_position}.old_position From 76dcccb7de501a85f2a0251a72fa0a5d0d99db81 Mon Sep 17 00:00:00 2001 From: BrokenEagle Date: Sat, 8 Feb 2020 06:46:58 +0000 Subject: [PATCH 02/10] Alter previous method on all versions models This is to prevent redoing the same SQL query which wasn't being cached. --- app/models/artist_commentary_version.rb | 7 +++++++ app/models/artist_version.rb | 5 ++++- app/models/note_version.rb | 5 ++++- app/models/pool_archive.rb | 5 ++++- app/models/post_archive.rb | 15 +++++++++------ app/models/wiki_page_version.rb | 5 ++++- 6 files changed, 32 insertions(+), 10 deletions(-) diff --git a/app/models/artist_commentary_version.rb b/app/models/artist_commentary_version.rb index 0f90027d9..8ce732caa 100644 --- a/app/models/artist_commentary_version.rb +++ b/app/models/artist_commentary_version.rb @@ -7,4 +7,11 @@ class ArtistCommentaryVersion < ApplicationRecord q = q.search_attributes(params, :post, :updater, :original_title, :original_description, :translated_title, :translated_description) q.apply_default_order(params) end + + def previous + @previous ||= begin + ArtistCommentaryVersion.where("post_id = ? and updated_at < ?", post_id, updated_at).order("updated_at desc").limit(1).to_a + end + @previous.first + end end diff --git a/app/models/artist_version.rb b/app/models/artist_version.rb index bfd9a92a9..8f015064b 100644 --- a/app/models/artist_version.rb +++ b/app/models/artist_version.rb @@ -24,6 +24,9 @@ class ArtistVersion < ApplicationRecord extend SearchMethods def previous - ArtistVersion.where("artist_id = ? and created_at < ?", artist_id, created_at).order("created_at desc").first + @previous ||= begin + ArtistVersion.where("artist_id = ? and created_at < ?", artist_id, created_at).order("created_at desc").limit(1).to_a + end + @previous.first end end diff --git a/app/models/note_version.rb b/app/models/note_version.rb index 231cb73f4..3cc1c56ea 100644 --- a/app/models/note_version.rb +++ b/app/models/note_version.rb @@ -13,6 +13,9 @@ class NoteVersion < ApplicationRecord end def previous - NoteVersion.where("note_id = ? and updated_at < ?", note_id, updated_at).order("updated_at desc").first + @previous ||= begin + NoteVersion.where("note_id = ? and updated_at < ?", note_id, updated_at).order("updated_at desc").limit(1).to_a + end + @previous.first end end diff --git a/app/models/pool_archive.rb b/app/models/pool_archive.rb index 8bb69f611..93c080844 100644 --- a/app/models/pool_archive.rb +++ b/app/models/pool_archive.rb @@ -108,7 +108,10 @@ class PoolArchive < ApplicationRecord end def previous - PoolArchive.where("pool_id = ? and version < ?", pool_id, version).order("version desc").first + @previous ||= begin + PoolArchive.where("pool_id = ? and version < ?", pool_id, version).order("version desc").limit(1).to_a + end + @previous.first end def pretty_name diff --git a/app/models/post_archive.rb b/app/models/post_archive.rb index 83fdd3fed..3663ac03a 100644 --- a/app/models/post_archive.rb +++ b/app/models/post_archive.rb @@ -92,13 +92,16 @@ class PostArchive < ApplicationRecord end def previous - # HACK: if all the post versions for this post have already been preloaded, - # we can use that to avoid a SQL query. - if association(:post).loaded? && post && post.association(:versions).loaded? - post.versions.sort_by(&:version).reverse.find { |v| v.version < version } - else - PostArchive.where("post_id = ? and version < ?", post_id, version).order("version desc").first + @previous ||= begin + # HACK: if all the post versions for this post have already been preloaded, + # we can use that to avoid a SQL query. + if association(:post).loaded? && post && post.association(:versions).loaded? + ver = [post.versions.sort_by(&:version).reverse.find { |v| v.version < version }] + else + ver = PostArchive.where("post_id = ? and version < ?", post_id, version).order("version desc").limit(1).to_a + end end + @previous.first end def visible? diff --git a/app/models/wiki_page_version.rb b/app/models/wiki_page_version.rb index 3dbdec763..ba69541a5 100644 --- a/app/models/wiki_page_version.rb +++ b/app/models/wiki_page_version.rb @@ -23,7 +23,10 @@ class WikiPageVersion < ApplicationRecord end def previous - WikiPageVersion.where("wiki_page_id = ? and id < ?", wiki_page_id, id).order("id desc").first + @previous ||= begin + WikiPageVersion.where("wiki_page_id = ? and id < ?", wiki_page_id, id).order("id desc").limit(1).to_a + end + @previous.first end def category_name From 434e031faa52e8105465a1f6efee410cffd2635b Mon Sep 17 00:00:00 2001 From: BrokenEagle Date: Sat, 8 Feb 2020 22:37:55 +0000 Subject: [PATCH 03/10] Add additional helper methods --- app/helpers/application_helper.rb | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index c7cd7d9d2..21b5e2b4c 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -1,11 +1,38 @@ require 'dtext' module ApplicationHelper + def listing_type(*fields, member_check: true, types: [:revert, :standard]) + (fields.reduce(false) { |acc, field| acc || params.dig(:search, field).present? } && (!member_check || CurrentUser.is_member?) ? types[0] : types[1]) + end + def diff_list_html(new, old, latest, ul_class: ["diff-list"], li_class: []) diff = SetDiff.new(new, old, latest) render "diff_list", diff: diff, ul_class: ul_class, li_class: li_class end + def diff_body_html(record, previous, field) + return h(record[field]).gsub(/\r?\n/, '
').html_safe if previous.blank? + + pattern = Regexp.new('(?:<.+?>)|(?:\w+)|(?:[ \t]+)|(?:\r?\n)|(?:.+?)') + DiffBuilder.new(record[field], previous[field], pattern).build + end + + def status_diff_html(record) + previous = record.previous + + return "New" if previous.blank? + + statuses = [] + record.class.status_fields.each do |field, status| + if record.has_attribute?(field) + statuses += [status] if record[field] != previous[field] + else + statuses += [status] if record.send(field) + end + end + statuses.join("
").html_safe + end + def wordbreakify(string) lines = string.scan(/.{1,10}/) wordbreaked_string = lines.map {|str| h(str)}.join("") From 154849a501a176a4c917b4607756ebe1876b1f1a Mon Sep 17 00:00:00 2001 From: BrokenEagle Date: Sat, 8 Feb 2020 16:21:37 +0000 Subject: [PATCH 04/10] Added/modified common stylesheets - Added a generalized diff-body class - Added a generalized versions stylesheet --- app/javascript/src/styles/common/diffs.scss | 16 ++++++++++++++++ app/javascript/src/styles/common/versions.scss | 13 +++++++++++++ 2 files changed, 29 insertions(+) create mode 100644 app/javascript/src/styles/common/versions.scss diff --git a/app/javascript/src/styles/common/diffs.scss b/app/javascript/src/styles/common/diffs.scss index 43a5bc9b3..f126b2e6d 100644 --- a/app/javascript/src/styles/common/diffs.scss +++ b/app/javascript/src/styles/common/diffs.scss @@ -39,3 +39,19 @@ color: var(--diff-list-obsolete-removed-color); } } + +.diff-body { + del { + background: var(--wiki-page-versions-diff-del-background); + text-decoration: none; + } + + ins { + background: var(--wiki-page-versions-diff-ins-background); + text-decoration: none; + } + + span.paragraph-mark { + opacity: 0.25; + } +} diff --git a/app/javascript/src/styles/common/versions.scss b/app/javascript/src/styles/common/versions.scss new file mode 100644 index 000000000..5913cc2b1 --- /dev/null +++ b/app/javascript/src/styles/common/versions.scss @@ -0,0 +1,13 @@ +body.a-index { + div#p-revert-listing { + display: flex; + + table.striped { + flex: 1; + } + } + + div#p-revert-listing > article.post-preview { + margin-top: 2em; + } +} From d8fd1c212e8cd7c54f5c1d01bfab34df2229a903 Mon Sep 17 00:00:00 2001 From: BrokenEagle Date: Fri, 7 Feb 2020 23:23:48 +0000 Subject: [PATCH 05/10] Revise display on note versions index - Body now uses the diff builder to highlight changes -- A generalized diff-body class was added instead of something specific - The status changes are now verbalized instead of being shown with styles - The position and sizes are now split up -- Changes directly reference the previous version - The date and user columns were combined -- This is more in line with other indexes, plus it saves space --- app/helpers/note_versions_helper.rb | 35 ++++++++--------------- app/models/note_version.rb | 26 +++++++++++++++++ app/views/note_versions/_listing.html.erb | 34 ++++++++++++---------- 3 files changed, 57 insertions(+), 38 deletions(-) diff --git a/app/helpers/note_versions_helper.rb b/app/helpers/note_versions_helper.rb index c29996ef0..e020b5c45 100644 --- a/app/helpers/note_versions_helper.rb +++ b/app/helpers/note_versions_helper.rb @@ -1,34 +1,23 @@ module NoteVersionsHelper - def note_versions_listing_type - ((params.dig(:search, :post_id).present? || params.dig(:search, :note_id).present?) && CurrentUser.is_member?) ? :revert : :standard - end - - def note_version_body_diff_info(note_version) - previous = note_version.previous - if previous.nil? - return "" - end - - html = "" - if note_version.body == previous.body - html += '(body not changed)' - end - - html.html_safe - end - def note_version_position_diff(note_version) previous = note_version.previous - html = "#{note_version.width}x#{note_version.height}" - html += " #{note_version.x},#{note_version.y}" + html = "#{note_version.x},#{note_version.y}" if previous.nil? html - elsif note_version.x == previous.x && note_version.y == previous.y && note_version.width == previous.width && note_version.height == previous.height + else + "#{previous.x},#{previous.y} -> " + html + end + end + + def note_version_size_diff(note_version) + previous = note_version.previous + + html = "#{note_version.width}x#{note_version.height}" + if previous.nil? html else - html = '' + html + '' - html.html_safe + "#{previous.width}x#{previous.height} -> " + html end end end diff --git a/app/models/note_version.rb b/app/models/note_version.rb index 3cc1c56ea..0e65e5738 100644 --- a/app/models/note_version.rb +++ b/app/models/note_version.rb @@ -18,4 +18,30 @@ class NoteVersion < ApplicationRecord end @previous.first end + + def self.status_fields + { + body: "Body", + was_moved: "Moved", + was_resized: "Resized", + was_deleted: "Deleted", + was_undeleted: "Undeleted", + } + end + + def was_moved + x != previous.x || y != previous.y + end + + def was_resized + width != previous.width || height != previous.height + end + + def was_deleted + !is_active && previous.is_active + end + + def was_undeleted + is_active && !previous.is_active + end end diff --git a/app/views/note_versions/_listing.html.erb b/app/views/note_versions/_listing.html.erb index eb95d5c6e..8449b678d 100644 --- a/app/views/note_versions/_listing.html.erb +++ b/app/views/note_versions/_listing.html.erb @@ -1,35 +1,39 @@ -
+
<%= table_for @note_versions, {class: "striped autofit", width: "100%"} do |t| %> <% t.column "Post", width: "5%" do |note_version| %> <%= link_to note_version.post_id, post_path(note_version.post_id) %> <% if !params.dig(:search, :post_id).present? %> - <%= link_to "»", note_versions_path(search: {post_id: note_version.post_id}) %> + <%= link_to "»", note_versions_path(search: {post_id: note_version.post_id}, anchor: "note-version-#{note_version.id}") %> <% end %> <% end %> <% t.column "Note", width: "5%" do |note_version| %> <%= link_to "#{note_version.note_id}.#{note_version.version}", post_path(note_version.post_id, anchor: "note-#{note_version.note_id}") %> <% if !params.dig(:search, :note_id).present? %> - <%= link_to "»", note_versions_path(search: {note_id: note_version.note_id}) %> + <%= link_to "»", note_versions_path(search: {note_id: note_version.note_id}, anchor: "note-version-#{note_version.id}") %> <% end %> <% end %> - <% t.column "Body", td: {class: "col-expand"} do |note_version| %> - <%= h(note_version.body) %> - <% unless note_version.is_active? %> - (deleted) - <% end %> - <%= note_version_body_diff_info(note_version) %> + <% t.column "Body", td: {class: "col-expand diff-body"} do |note_version| %> + <%= diff_body_html(note_version, note_version.previous, :body) %> <% end %> - <% t.column "Position", width: "5%" do |note_version| %> + <% t.column "Position (X,Y)", width: "5%", column: "position" do |note_version| %> <%= note_version_position_diff(note_version) %> <% end %> - <% t.column "Edited By", width: "10%" do |note_version| %> + <% t.column "Size (WxH)", width: "5%", column: "size" do |note_version| %> + <%= note_version_size_diff(note_version) %> + <% end %> + <% t.column "Changes", width: "3%" do |note_version| %> + <%= status_diff_html(note_version) %> + <% end %> + <% t.column "Updated", width: "10%" do |note_version| %> +
+ <%= compact_time note_version.updated_at %> +
+ by <%= link_to_user note_version.updater %> + <%= link_to "»", note_versions_path(search: params[:search].merge({ updater_id: note_version.updater_id })) %> <% end %> - <% t.column "Date", width: "10%" do |note_version| %> - <%= compact_time note_version.updated_at %> - <% end %> - <% if note_versions_listing_type == :revert %> + <% if listing_type(:post_id, :note_id) == :revert %> <% t.column column: "control", width: "7%" do |note_version| %> <%= link_to "Revert to", revert_note_path(note_version.note_id, :version_id => note_version.id), :remote => true, :method => :put, :data => {:confirm => "Are you sure you want to revert to this version?"} %> <% end %> From d62f7e786e7ec06f6d78561d69ef3d81e5c3d4d0 Mon Sep 17 00:00:00 2001 From: BrokenEagle Date: Sat, 8 Feb 2020 03:58:26 +0000 Subject: [PATCH 06/10] Revise display on artist commentary versions index - All text fields are now shown in their non-rendered form -- This allows changes to be highlighted with the diff builder -- The different fields were labeled and separated for easier discernment -- Fields are only shown if they have text in either the current or previous versions - Various changes are also verbalized for easier discovery - The date and the user columns were combined -- This is more in line with other indexes, plus it saves on space - The revert listing was changed to use a thumbnail instead of post ID links -- This makes it more in line with the post versions index --- .../artist_commentary_versions_helper.rb | 5 -- .../specific/artist_commentary_versions.scss | 15 ++++ app/models/artist_commentary_version.rb | 13 ++++ .../_listing.html.erb | 69 +++++++++++++------ 4 files changed, 76 insertions(+), 26 deletions(-) delete mode 100644 app/helpers/artist_commentary_versions_helper.rb create mode 100644 app/javascript/src/styles/specific/artist_commentary_versions.scss diff --git a/app/helpers/artist_commentary_versions_helper.rb b/app/helpers/artist_commentary_versions_helper.rb deleted file mode 100644 index 0a52fed5d..000000000 --- a/app/helpers/artist_commentary_versions_helper.rb +++ /dev/null @@ -1,5 +0,0 @@ -module ArtistCommentaryVersionsHelper - def artist_commentary_versions_listing_type - params.dig(:search, :post_id).present? ? :revert : :standard - end -end diff --git a/app/javascript/src/styles/specific/artist_commentary_versions.scss b/app/javascript/src/styles/specific/artist_commentary_versions.scss new file mode 100644 index 000000000..c60410940 --- /dev/null +++ b/app/javascript/src/styles/specific/artist_commentary_versions.scss @@ -0,0 +1,15 @@ +div#c-artist-commentary-versions { + #a-index { + div.commentary-body-section { + padding: 0.5em; + margin-bottom: 0.5em; + border: var(--footer-border); + } + + td.original-column, + td.translated-column { + padding-top: 0.5em; + vertical-align: top; + } + } +} diff --git a/app/models/artist_commentary_version.rb b/app/models/artist_commentary_version.rb index 8ce732caa..e362821cb 100644 --- a/app/models/artist_commentary_version.rb +++ b/app/models/artist_commentary_version.rb @@ -14,4 +14,17 @@ class ArtistCommentaryVersion < ApplicationRecord end @previous.first end + + def self.status_fields + { + original_title: "OrigTitle", + original_description: "OrigDesc", + translated_title: "TransTitle", + translated_description: "TransDesc", + } + end + + def unchanged_empty?(field) + self[field].strip.empty? && (previous.nil? || previous[field].strip.empty?) + end end diff --git a/app/views/artist_commentary_versions/_listing.html.erb b/app/views/artist_commentary_versions/_listing.html.erb index 2f2e1f18d..fd4fb089d 100644 --- a/app/views/artist_commentary_versions/_listing.html.erb +++ b/app/views/artist_commentary_versions/_listing.html.erb @@ -1,33 +1,60 @@ -
+
- <%= table_for @commentary_versions, {class: "striped autofit", width: "100%"} do |t| %> - <% t.column "Post", width: "5%" do |commentary_version| %> - <% if artist_commentary_versions_listing_type == :revert %> - <%= link_to commentary_version.post_id, post_path(commentary_version.post_id) %> - <% else %> - <%= PostPresenter.preview(commentary_version.post, :tags => "status:any") %> + <% if listing_type(:post_id) == :revert %> + <%= PostPresenter.preview(@commentary_versions.first.post, show_deleted: true) %> + <% end %> + + <%= table_for @commentary_versions, {class: "striped", width: "100%"} do |t| %> + <% if listing_type(:post_id) == :standard %> + <% t.column "Post", width: "1%" do |commentary_version| %> + <%= PostPresenter.preview(commentary_version.post, :tags => "status:any") %> <% end %> <% end %> - <% if artist_commentary_versions_listing_type == :standard %> - <% t.column "Version" do |commentary_version| %> - <%= link_to "#{commentary_version.post_id}.#{commentary_version.id}»", artist_commentary_versions_path(search: {post_id: commentary_version.post_id}) %> + <% if listing_type(:post_id) == :standard %> + <% t.column "Version", width: "3%" do |commentary_version| %> + <%= link_to "#{commentary_version.post_id}.#{commentary_version.id}»", artist_commentary_versions_path(search: {post_id: commentary_version.post_id}, anchor: "artist-commentary-version-#{commentary_version.id}") %> <% end %> <% end %> - <% t.column "Original" do |commentary_version| %> - <%= format_commentary_title(commentary_version.original_title) %> - <%= format_commentary_description(commentary_version.original_description) %> + <% t.column "Original", width: "40%", td: {class: "diff-body"} do |commentary_version| %> + <% if !commentary_version.unchanged_empty?(:original_title) %> + Title: +
+ <%= diff_body_html(commentary_version, commentary_version.previous, :original_title) %> +
+ <% end %> + <% if !commentary_version.unchanged_empty?(:original_description) %> + Description: +
+ <%= diff_body_html(commentary_version, commentary_version.previous, :original_description) %> +
+ <% end %> <% end %> - <% t.column "Translated" do |commentary_version| %> - <%= format_commentary_title(commentary_version.translated_title) %> - <%= format_commentary_description(commentary_version.translated_description) %> + <% t.column "Translated", width: "40%", td: {class: "diff-body"} do |commentary_version| %> + <% if !commentary_version.unchanged_empty?(:translated_title) %> + Title: +
+ <%= diff_body_html(commentary_version, commentary_version.previous, :translated_title) %> +
+ <% end %> + <% if !commentary_version.unchanged_empty?(:translated_description) %> + Description: +
+ <%= diff_body_html(commentary_version, commentary_version.previous, :translated_description) %> +
+ <% end %> <% end %> - <% t.column "Edited by", width: "10%" do |commentary_version| %> + <% t.column "Changes", width: "3%" do |commentary_version| %> + <%= status_diff_html(commentary_version) %> + <% end %> + <% t.column "Updated", width: "10%" do |commentary_version| %> +
+ <%= compact_time commentary_version.updated_at %> +
+ by <%= link_to_user commentary_version.updater %> + <%= link_to "»", artist_commentary_versions_path(search: params[:search].merge({ updater_id: commentary_version.updater_id })) %> <% end %> - <% t.column "Date", width: "10%" do |commentary_version| %> - <%= compact_time commentary_version.updated_at %> - <% end %> - <% if artist_commentary_versions_listing_type == :revert %> + <% if listing_type(:post_id) == :revert %> <% t.column column: "control", width: "7%" do |commentary_version| %> <%= link_to "Revert to", revert_artist_commentary_path(commentary_version.post_id, :version_id => commentary_version.id), :remote => true, :method => :put, :data => {:confirm => "Are you sure you want to revert to this version?"} %> <% end %> From ede7167bb89a9a3ecccbb7adf30a38393482f790 Mon Sep 17 00:00:00 2001 From: BrokenEagle Date: Sat, 8 Feb 2020 06:49:31 +0000 Subject: [PATCH 07/10] Rework pool versions index/diff views - Changed to using the diff-body CSS class -- Removed unneeded CSS style file - Removed trailing whitespace after the >>> link -- It was causing artifact line-throughs to appear after the link - Changed the diff link to only render when a text field has changed -- Because the post changes are already shown on the index view - Specifically add
to statuses to cause line breaks --- app/helpers/pool_versions_helper.rb | 26 ---------------- .../src/styles/specific/pool_versions.scss | 17 ---------- app/models/pool_archive.rb | 31 +++++++++++++++++++ app/views/pool_versions/_diff.html.erb | 4 +-- app/views/pool_versions/_listing.html.erb | 27 ++++++++-------- app/views/pool_versions/diff.html.erb | 6 ++-- 6 files changed, 49 insertions(+), 62 deletions(-) delete mode 100644 app/helpers/pool_versions_helper.rb delete mode 100644 app/javascript/src/styles/specific/pool_versions.scss diff --git a/app/helpers/pool_versions_helper.rb b/app/helpers/pool_versions_helper.rb deleted file mode 100644 index 4c6fe01e8..000000000 --- a/app/helpers/pool_versions_helper.rb +++ /dev/null @@ -1,26 +0,0 @@ -module PoolVersionsHelper - def pool_versions_listing_type - params.dig(:search, :pool_id).present? ? :revert : :standard - end - - def pool_version_status_diff(pool_version) - cur = pool_version - prev = pool_version.previous - - return "New" if prev.blank? - - status = [] - status += ["Renamed"] if cur.name != prev.name - status += ["DescChanged"] if cur.description != prev.description - status += ["Deleted"] if cur.is_deleted? && !prev.is_deleted? - status += ["Undeleted"] if !cur.is_deleted? && prev.is_deleted? - status += ["Activated"] if cur.is_active? && !prev.is_active? - status += ["Deactivated"] if !cur.is_active? && prev.is_active? - status.join(" ") - end - - def pool_page_diff(pool_version, other_version) - pattern = Regexp.new('(?:<.+?>)|(?:\w+)|(?:[ \t]+)|(?:\r?\n)|(?:.+?)') - DiffBuilder.new(pool_version.description, other_version.description, pattern).build - end -end diff --git a/app/javascript/src/styles/specific/pool_versions.scss b/app/javascript/src/styles/specific/pool_versions.scss deleted file mode 100644 index 1d7d6c8e7..000000000 --- a/app/javascript/src/styles/specific/pool_versions.scss +++ /dev/null @@ -1,17 +0,0 @@ -div#c-pool-versions { - #a-diff { - del { - background: var(--wiki-page-versions-diff-del-background); - text-decoration: none; - } - - ins { - background: var(--wiki-page-versions-diff-ins-background); - text-decoration: none; - } - - span.paragraph-mark { - opacity: 0.25; - } - } -} \ No newline at end of file diff --git a/app/models/pool_archive.rb b/app/models/pool_archive.rb index 93c080844..f2fe3b031 100644 --- a/app/models/pool_archive.rb +++ b/app/models/pool_archive.rb @@ -114,6 +114,37 @@ class PoolArchive < ApplicationRecord @previous.first end + def self.status_fields + { + name: "Renamed", + description: "Description", + was_deleted: "Deleted", + was_undeleted: "Undeleted", + was_activated: "Activated", + was_deactivated: "Deactivated", + } + end + + def was_deleted + is_deleted && !previous.is_deleted + end + + def was_undeleted + !is_deleted && previous.is_deleted + end + + def was_activated + is_active && !previous.is_active + end + + def was_deactivated + !is_active && previous.is_active + end + + def text_field_changed + previous.present? && (name_changed || description_changed) + end + def pretty_name name.tr("_", " ") end diff --git a/app/views/pool_versions/_diff.html.erb b/app/views/pool_versions/_diff.html.erb index 543d8c42a..83e90ec02 100644 --- a/app/views/pool_versions/_diff.html.erb +++ b/app/views/pool_versions/_diff.html.erb @@ -11,7 +11,7 @@ <% diff[:removed_post_ids].each do |post_id| %> <%= link_to post_id, post_path(post_id) %><%# - %><%= link_to "»", pool_versions_path(search: { post_id: post_id }) %> - + %><%= link_to "»", pool_versions_path(search: { post_id: post_id }) %><%# + %> <% end %> diff --git a/app/views/pool_versions/_listing.html.erb b/app/views/pool_versions/_listing.html.erb index 1a4ea0ba0..82462ed36 100644 --- a/app/views/pool_versions/_listing.html.erb +++ b/app/views/pool_versions/_listing.html.erb @@ -1,12 +1,12 @@ -
+
<%= table_for @pool_versions, {class: "striped autofit", width: "100%"} do |t| %> <% t.column column: "diff", width: "3%" do |pool_version| %> - <%= link_to_if pool_version.previous.present?, "diff", diff_pool_version_path(pool_version.id) %> + <%= link_to_if pool_version.text_field_changed, "diff", diff_pool_version_path(pool_version.id) %> <% end %> <% t.column "Pool" do |pool_version| %> <%= link_to pool_version.pretty_name, pool_path(pool_version.pool_id), class: "pool-category-#{pool_version.pool.category}" %> - <%= link_to "»", pool_versions_path(search: { pool_id: pool_version.pool_id }), class: "pool-category-#{pool_version.pool.category}" %> + <%= link_to "»", pool_versions_path(search: { pool_id: pool_version.pool_id }, anchor: "pool-archive-#{pool_version.id}"), class: "pool-category-#{pool_version.pool.category}" %> <% end %> <% t.column "Post Changes", td: { class: "col-expand" } do |pool_version| %> <%= render "pool_versions/diff", diff: pool_version.build_diff %> @@ -14,19 +14,18 @@ <% t.column "Post Count" do |pool_version| %> <%= link_to pool_version.post_ids.size, pool_versions_path(search: { pool_id: pool_version.pool_id }) %> <% end %> - <% t.column "Status", td: {class: "col-expand"} do |pool_version| %> - <%= pool_version_status_diff(pool_version) %> + <% t.column "Changes", td: {class: "col-expand"} do |pool_version| %> + <%= status_diff_html(pool_version) %> <% end %> - <% t.column "Updater" do |pool_version| %> - <% if pool_version.updater %> - <%= link_to_user pool_version.updater %> - <%= link_to "»", pool_versions_path(search: { updater_id: pool_version.updater_id }) %> - <% end %> + <% t.column "Updated", width: "10%" do |pool_version| %> +
+ <%= compact_time pool_version.updated_at %> +
+ by + <%= link_to_user pool_version.updater %> + <%= link_to "»", pool_versions_path(search: params[:search].merge({ updater_id: pool_version.updater_id })) %> <% end %> - <% t.column "Date" do |pool_version| %> - <%= compact_time pool_version.updated_at %> - <% end %> - <% if pool_versions_listing_type == :revert %> + <% if listing_type(:pool_id) == :revert %> <% t.column column: "control" do |pool_version| %> <%= link_to "Revert to", revert_pool_path(pool_version.pool_id, :version_id => pool_version.id), :method => :put, :remote => true %> <% end %> diff --git a/app/views/pool_versions/diff.html.erb b/app/views/pool_versions/diff.html.erb index 671b36d29..4d89b4707 100644 --- a/app/views/pool_versions/diff.html.erb +++ b/app/views/pool_versions/diff.html.erb @@ -7,7 +7,7 @@ <% if @other_version.present? %>

Showing differences between <%= compact_time @pool_version.updated_at %> (<%= link_to_user @pool_version.updater %>) and <%= compact_time @other_version.updated_at %> (<%= link_to_user @other_version.updater %>)

-
+

Name:

<% if @pool_version.name != @other_version.name %> @@ -21,11 +21,11 @@

Posts:

<%= render "pool_versions/diff", diff: @pool_version.build_diff(@other_version) %>

-
+

Description:

<% if @pool_version.description != @other_version.description %> - <%= pool_page_diff(@pool_version, @other_version) %> + <%= diff_body_html(@pool_version, @other_version, :description) %> <% else %> Unchanged. <% end %> From de1324098d4ed0d428a196d3440333079508a50e Mon Sep 17 00:00:00 2001 From: BrokenEagle Date: Sat, 8 Feb 2020 06:56:01 +0000 Subject: [PATCH 08/10] Rework wiki page versions index/diff views - Changed to using the diff-body CSS class -- Removed unnecessary elements from the CSS style file - Does a symmetric difference on the array fields to detect differences - Add more descriptors to the status/changes column - Specifically add
to statuses to cause line breaks --- app/helpers/wiki_page_versions_helper.rb | 22 ------------------- .../styles/specific/wiki_page_versions.scss | 10 +++------ app/models/wiki_page_version.rb | 22 +++++++++++++++++++ .../wiki_page_versions/_listing.html.erb | 12 +++++----- app/views/wiki_page_versions/diff.html.erb | 4 ++-- 5 files changed, 33 insertions(+), 37 deletions(-) diff --git a/app/helpers/wiki_page_versions_helper.rb b/app/helpers/wiki_page_versions_helper.rb index 88a4b110d..0dd614045 100644 --- a/app/helpers/wiki_page_versions_helper.rb +++ b/app/helpers/wiki_page_versions_helper.rb @@ -1,21 +1,4 @@ module WikiPageVersionsHelper - def wiki_page_versions_listing_type - params.dig(:search, :wiki_page_id).present? ? :page : :global - end - - def wiki_page_version_status_diff(wiki_page_version) - cur = wiki_page_version - prev = wiki_page_version.previous - - return "New" if prev.blank? - - status = [] - status += ["Renamed"] if cur.title != prev.title - status += ["Deleted"] if cur.is_deleted? && !prev.is_deleted? - status += ["Undeleted"] if !cur.is_deleted? && prev.is_deleted? - status.join(" ") - end - def wiki_other_names_diff(new_version, old_version) new_names = new_version.other_names old_names = old_version.other_names @@ -23,9 +6,4 @@ module WikiPageVersionsHelper diff_list_html(new_names, old_names, latest_names, ul_class: ["wiki-other-names-diff-list list-inline"], li_class: ["wiki-other-name"]) end - - def wiki_body_diff(thispage, otherpage) - pattern = Regexp.new('(?:<.+?>)|(?:\w+)|(?:[ \t]+)|(?:\r?\n)|(?:.+?)') - DiffBuilder.new(thispage.body, otherpage.body, pattern).build - end end diff --git a/app/javascript/src/styles/specific/wiki_page_versions.scss b/app/javascript/src/styles/specific/wiki_page_versions.scss index 15921873e..c83da2c0f 100644 --- a/app/javascript/src/styles/specific/wiki_page_versions.scss +++ b/app/javascript/src/styles/specific/wiki_page_versions.scss @@ -1,22 +1,18 @@ div#c-wiki-page-versions { #a-diff { - del, .wiki-other-names-diff-list .removed { + ul.wiki-other-names-diff-list li.removed { background: var(--wiki-page-versions-diff-del-background); text-decoration: none; } - ins, .wiki-other-names-diff-list .added { + ul.wiki-other-names-diff-list li.added { background: var(--wiki-page-versions-diff-ins-background); text-decoration: none; } - .wiki-other-names-diff-list .obsolete { + ul.wiki-other-names-diff-list li.obsolete { text-decoration: dotted underline; } - - span.paragraph-mark { - opacity: 0.25; - } } #a-index { diff --git a/app/models/wiki_page_version.rb b/app/models/wiki_page_version.rb index ba69541a5..e5d03ccfd 100644 --- a/app/models/wiki_page_version.rb +++ b/app/models/wiki_page_version.rb @@ -29,6 +29,28 @@ class WikiPageVersion < ApplicationRecord @previous.first end + def self.status_fields + { + body: "Body", + other_names_changed: "OtherNames", + title: "Renamed", + was_deleted: "Deleted", + was_undeleted: "Undeleted", + } + end + + def other_names_changed + ((other_names - previous.other_names) | (previous.other_names - other_names)).length > 0 + end + + def was_deleted + is_deleted && !previous.is_deleted + end + + def was_undeleted + !is_deleted && previous.is_deleted + end + def category_name Tag.category_for(title) end diff --git a/app/views/wiki_page_versions/_listing.html.erb b/app/views/wiki_page_versions/_listing.html.erb index d07336db9..3edd37d2a 100644 --- a/app/views/wiki_page_versions/_listing.html.erb +++ b/app/views/wiki_page_versions/_listing.html.erb @@ -1,11 +1,11 @@ -

+
<%= form_tag(diff_wiki_page_versions_path, :method => :get) do %> <%= table_for @wiki_page_versions, width: "100%" do |t| %> <% t.column column: "diff", width: "3%" do |wiki_page_version, i| %> <%= link_to_if wiki_page_version.previous.present?, "diff", diff_wiki_page_versions_path(otherpage: wiki_page_version.previous.try(:id), thispage: wiki_page_version.id) %> <% end %> - <% if wiki_page_versions_listing_type == :page %> + <% if listing_type(:wiki_page_id, member_check: false, types: [:page, :global]) == :page %> <% t.column column: "this-page", width: "2%" do |wiki_page_version, i| %> <%= radio_button_tag "thispage", wiki_page_version.id, (i == 1) %> <% end %> @@ -21,10 +21,10 @@ <%= link_to "»", wiki_page_versions_path(search: { wiki_page_id: wiki_page_version.wiki_page_id }) %> <% end %> - <% t.column "Status", width: "5%" do |wiki_page_version| %> - <%= wiki_page_version_status_diff(wiki_page_version) %> + <% t.column "Changes", width: "5%" do |wiki_page_version| %> + <%= status_diff_html(wiki_page_version) %> <% end %> - <% t.column "Last edited", width: "26%" do |wiki_page_version| %> + <% t.column "Updated", width: "26%" do |wiki_page_version| %> <%= compact_time(wiki_page_version.updated_at) %> by <%= link_to_user wiki_page_version.updater %> @@ -32,7 +32,7 @@ <% end %> <% end %> - <% if wiki_page_versions_listing_type == :page %> + <% if listing_type(:wiki_page_id, member_check: false, types: [:page, :global]) == :page %> <%= submit_tag "Diff" %> <% end %> <% end %> diff --git a/app/views/wiki_page_versions/diff.html.erb b/app/views/wiki_page_versions/diff.html.erb index ca5d2be33..5f749c1f8 100644 --- a/app/views/wiki_page_versions/diff.html.erb +++ b/app/views/wiki_page_versions/diff.html.erb @@ -10,7 +10,7 @@ <%= wiki_other_names_diff(@thispage, @otherpage) %> -
- <%= wiki_body_diff(@thispage, @otherpage) %> +
+ <%= diff_body_html(@thispage, @otherpage, :body) %>
<% end %> From 7b1efd12049be9f59ff16afcf9572765bb4ac903 Mon Sep 17 00:00:00 2001 From: BrokenEagle Date: Sat, 8 Feb 2020 08:25:36 +0000 Subject: [PATCH 09/10] Rework artist versions index view - Added a changes column explicitly listing all of the changes -- This makes it more in line with the other version views now - Does a symmetric difference on the array fields to detect changes --- app/helpers/artist_versions_helper.rb | 4 --- app/models/artist_version.rb | 37 +++++++++++++++++++++ app/views/artist_versions/_listing.html.erb | 20 +++++------ 3 files changed, 46 insertions(+), 15 deletions(-) diff --git a/app/helpers/artist_versions_helper.rb b/app/helpers/artist_versions_helper.rb index acd1c685e..1f55b471e 100644 --- a/app/helpers/artist_versions_helper.rb +++ b/app/helpers/artist_versions_helper.rb @@ -1,8 +1,4 @@ module ArtistVersionsHelper - def artist_versions_listing_type - (params.dig(:search, :artist_id).present? && CurrentUser.is_member?) ? :revert : :standard - end - def artist_version_other_names_diff(artist_version) new_names = artist_version.other_names old_names = artist_version.previous.try(:other_names) diff --git a/app/models/artist_version.rb b/app/models/artist_version.rb index 8f015064b..d8486cae3 100644 --- a/app/models/artist_version.rb +++ b/app/models/artist_version.rb @@ -29,4 +29,41 @@ class ArtistVersion < ApplicationRecord end @previous.first end + + def self.status_fields + { + name: "Renamed", + urls_changed: "URLs", + other_names_changed: "OtherNames", + group_name: "GroupName", + was_deleted: "Deleted", + was_undeleted: "Undeleted", + was_banned: "Banned", + was_unbanned: "Unbanned", + } + end + + def other_names_changed + ((other_names - previous.other_names) | (previous.other_names - other_names)).length > 0 + end + + def urls_changed + ((urls - previous.urls) | (previous.urls - urls)).length > 0 + end + + def was_deleted + !is_active && previous.is_active + end + + def was_undeleted + is_active && !previous.is_active + end + + def was_banned + is_banned && !previous.is_banned + end + + def was_unbanned + !is_banned && previous.is_banned + end end diff --git a/app/views/artist_versions/_listing.html.erb b/app/views/artist_versions/_listing.html.erb index 0275adff0..5051698cd 100644 --- a/app/views/artist_versions/_listing.html.erb +++ b/app/views/artist_versions/_listing.html.erb @@ -1,24 +1,22 @@ -
+
<%= table_for @artist_versions, {class: "striped autofit", width: "100%"} do |t| %> <% t.column "Name" do |artist_version| %> <%= link_to artist_version.name, artist_path(artist_version.artist_id) %> - <%= link_to "»", artist_versions_path(search: {artist_id: artist_version.artist_id}) %> - - <% if !artist_version.is_active? %> - (deleted) - <% end %> - - <% if artist_version.group_name.present? %> -

(group: <%= artist_version.group_name %>)

- <% end %> + <%= link_to "»", artist_versions_path(search: {artist_id: artist_version.artist_id}, anchor: "artist-version-#{artist_version.id}") %> <% end %> <% t.column "Other Names" do |artist_version| %> + <% if artist_version.group_name.present? %> +

Group:
 <%= artist_version.group_name %>

+ <% end %> <%= artist_version_other_names_diff(artist_version) %> <% end %> <% t.column "URLs", td: {class: "col-expand"} do |artist_version| %> <%= artist_version_urls_diff(artist_version) %> <% end %> + <% t.column "Changes" do |artist_version| %> + <%= status_diff_html(artist_version) %> + <% end %> <% t.column "Updated" do |artist_version| %> <%= link_to_user artist_version.updater %> <%= link_to "»", artist_versions_path(search: { updater_name: artist_version.updater.name }) %> @@ -26,7 +24,7 @@ <%= compact_time(artist_version.updated_at) %>

<% end %> - <% if artist_versions_listing_type == :revert %> + <% if listing_type(:artist_id) == :revert %> <% t.column column: "control" do |artist_version| %> <%= link_to "Revert to", revert_artist_path(artist_version.artist_id, version_id: artist_version.id), method: :put, "data-confirm": "Are you sure you want to revert to this version?" %> <% end %> From 8ff00cfc7d426b141fd9d2f00b19bfb9f683e29c Mon Sep 17 00:00:00 2001 From: BrokenEagle Date: Sat, 8 Feb 2020 08:57:26 +0000 Subject: [PATCH 10/10] Rework post version view - Added a changes column explicitly listing all of the changes -- This makes it more in line with the other views now --- app/helpers/post_versions_helper.rb | 4 ---- .../src/styles/specific/post_versions.scss | 5 ----- app/models/post_archive.rb | 9 +++++++++ app/views/post_versions/_listing.html.erb | 13 ++++++++----- app/views/post_versions/index.html.erb | 2 +- 5 files changed, 18 insertions(+), 15 deletions(-) diff --git a/app/helpers/post_versions_helper.rb b/app/helpers/post_versions_helper.rb index 3e354f7c1..062deebf4 100644 --- a/app/helpers/post_versions_helper.rb +++ b/app/helpers/post_versions_helper.rb @@ -1,8 +1,4 @@ module PostVersionsHelper - def post_versions_listing_type - params.dig(:search, :post_id).present? ? :revert : :standard - end - def post_version_diff(post_version) diff = post_version.diff(post_version.previous) html = '' diff --git a/app/javascript/src/styles/specific/post_versions.scss b/app/javascript/src/styles/specific/post_versions.scss index a52116b41..7c8bf1f9b 100644 --- a/app/javascript/src/styles/specific/post_versions.scss +++ b/app/javascript/src/styles/specific/post_versions.scss @@ -10,9 +10,4 @@ body.c-post-versions.a-index { .advanced-search-link { margin: 0 1em; } - - #p-revert-listing { - display: flex; - table#post-versions-table { flex: 1; } - } } diff --git a/app/models/post_archive.rb b/app/models/post_archive.rb index 3663ac03a..454475ca8 100644 --- a/app/models/post_archive.rb +++ b/app/models/post_archive.rb @@ -108,6 +108,15 @@ class PostArchive < ApplicationRecord post&.visible? end + def self.status_fields + { + tags: "Tags", + rating: "Rating", + parent_id: "Parent", + source: "Source", + } + end + def diff(version = nil) if post.nil? latest_tags = tag_array diff --git a/app/views/post_versions/_listing.html.erb b/app/views/post_versions/_listing.html.erb index a16600aba..b0997ff06 100644 --- a/app/views/post_versions/_listing.html.erb +++ b/app/views/post_versions/_listing.html.erb @@ -1,5 +1,5 @@ -
- <% if post_versions_listing_type == :revert %> +
+ <% if listing_type(:post_id) == :revert %> <%= PostPresenter.preview(@post_versions.first.post, show_deleted: true) %> <% end %> @@ -9,17 +9,20 @@ > <% end %> <% end %> - <% if post_versions_listing_type == :standard %> + <% if listing_type(:post_id) == :standard %> <% t.column "Post" 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}") %> + <%= link_to "#{post_version.post_id}.#{post_version.version}", post_versions_path(search: { post_id: post_version.post_id }, anchor: "post-archive-#{post_version.id}") %> <% end %> <% t.column "Tags", td: {class: "col-expand"} do |post_version| %> <%= post_version_diff(post_version) %> <% end %> + <% t.column "Changes" do |post_version| %> + <%= status_diff_html(post_version) %> + <% end %> <% t.column "Updated" do |post_version| %> <%= link_to_user post_version.updater %> <%= link_to "»", post_versions_path(search: params[:search].merge({ updater_name: post_version.updater&.name })) %> @@ -31,7 +34,7 @@ <% if post_version.can_undo?(CurrentUser.user) %> <%= link_to "Undo", undo_post_version_path(post_version), method: :put, remote: true, class: "post-version-undo-link" %> <% end %> - <% if post_versions_listing_type == :revert && post_version.can_revert_to?(CurrentUser.user) %> + <% if listing_type(:post_id) == :revert && post_version.can_revert_to?(CurrentUser.user) %> | <%= link_to "Revert to", revert_post_path(post_version.post_id, version_id: post_version.id), method: :put, remote: true %> <% end %> <% end %> diff --git a/app/views/post_versions/index.html.erb b/app/views/post_versions/index.html.erb index 6fc04c52b..a17e7e724 100644 --- a/app/views/post_versions/index.html.erb +++ b/app/views/post_versions/index.html.erb @@ -1,6 +1,6 @@
- <% if post_versions_listing_type == :revert && @post_versions.present? %> + <% if listing_type(:post_id) == :revert && @post_versions.present? %>

Tag History: <%= link_to "Post ##{params.dig(:search, :post_id)}", @post_versions[0].post %>

<% else %>

Tag History