From aece2b61a92b4e3aa315f76308b9f58b412f8fba Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Tue, 11 Mar 2014 18:51:26 +0100
Subject: [PATCH] FIX: revision history UI

---
 .../controllers/history_controller.js         | 22 +++------
 .../templates/modal/history.js.handlebars     | 10 ++--
 app/assets/stylesheets/desktop/history.scss   |  3 ++
 app/models/post_revision.rb                   | 49 ++++++++-----------
 4 files changed, 38 insertions(+), 46 deletions(-)

diff --git a/app/assets/javascripts/discourse/controllers/history_controller.js b/app/assets/javascripts/discourse/controllers/history_controller.js
index 62481df8c71..a4411e174c4 100644
--- a/app/assets/javascripts/discourse/controllers/history_controller.js
+++ b/app/assets/javascripts/discourse/controllers/history_controller.js
@@ -12,17 +12,11 @@ Discourse.HistoryController = Discourse.ObjectController.extend(Discourse.ModalF
   viewMode: "side_by_side",
 
   refresh: function(postId, postVersion) {
-    this.setProperties({
-      loading: true,
-      viewMode: Discourse.Mobile.mobileView ? "inline" : "side_by_side"
-    });
+    this.set("loading", true);
 
     var self = this;
     Discourse.Post.loadRevision(postId, postVersion).then(function (result) {
-      self.setProperties({
-        loading: false,
-        model: result
-      });
+      self.setProperties({ loading: false, model: result });
     });
   },
 
@@ -46,9 +40,9 @@ Discourse.HistoryController = Discourse.ObjectController.extend(Discourse.ModalF
     var curCategory = Discourse.Category.findById(changes.current_category_id);
 
     var raw = "";
-
-    prevCategory = Discourse.HTML.categoryLink(prevCategory);
-    curCategory = Discourse.HTML.categoryLink(curCategory);
+    var opts = { allowUncategorized: true };
+    prevCategory = Discourse.HTML.categoryLink(prevCategory, opts);
+    curCategory = Discourse.HTML.categoryLink(curCategory, opts);
 
     if(viewMode === "side_by_side_markdown" || viewMode === "side_by_side") {
       raw = "<div class='span8'>" + prevCategory +  "</div> <div class='span8 offset1'>" + curCategory +  "</div>";
@@ -64,7 +58,7 @@ Discourse.HistoryController = Discourse.ObjectController.extend(Discourse.ModalF
 
     return raw;
 
-  }.property("inline", "side_by_side", "side_by_side_markdown", "viewMode"),
+  }.property("viewMode", "category_changes"),
 
   title_diff: function() {
     var viewMode = this.get("viewMode");
@@ -72,11 +66,11 @@ Discourse.HistoryController = Discourse.ObjectController.extend(Discourse.ModalF
       viewMode = "side_by_side";
     }
     return this.get("title_changes." + viewMode);
-  }.property("inline", "side_by_side", "side_by_side_markdown", "viewMode"),
+  }.property("viewMode", "title_changes"),
 
   body_diff: function() {
     return this.get("body_changes." + this.get("viewMode"));
-  }.property("inline", "side_by_side", "side_by_side_markdown", "viewMode"),
+  }.property("viewMode", "body_changes"),
 
   actions: {
     loadFirstVersion: function() { this.refresh(this.get("post_id"), 2); },
diff --git a/app/assets/javascripts/discourse/templates/modal/history.js.handlebars b/app/assets/javascripts/discourse/templates/modal/history.js.handlebars
index e092e9a8a08..7415153602a 100644
--- a/app/assets/javascripts/discourse/templates/modal/history.js.handlebars
+++ b/app/assets/javascripts/discourse/templates/modal/history.js.handlebars
@@ -23,12 +23,14 @@
     </div>
     <div id="revisions">
       {{#if title_changes}}
-        <h2>{{{title_diff}}}</h2>
+        <div class="row">
+          <h2>{{{title_diff}}}</h2>
+        </div>
       {{/if}}
       {{#if category_changes}}
-      <div class="category-diff">
-        {{{category_diff}}}
-      </div>
+        <div class="row">
+          {{{category_diff}}}
+        </div>
       {{/if}}
       {{{body_diff}}}
     </div>
diff --git a/app/assets/stylesheets/desktop/history.scss b/app/assets/stylesheets/desktop/history.scss
index 56c358b131b..6aefebf9bab 100644
--- a/app/assets/stylesheets/desktop/history.scss
+++ b/app/assets/stylesheets/desktop/history.scss
@@ -24,6 +24,9 @@
   }
   #revisions {
     word-wrap: break-word;
+    .row, table {
+      margin-top: 10px;
+    }
   }
   img {
     max-width: 670px;
diff --git a/app/models/post_revision.rb b/app/models/post_revision.rb
index d3b74a37400..58bca9e96b5 100644
--- a/app/models/post_revision.rb
+++ b/app/models/post_revision.rb
@@ -7,42 +7,38 @@ class PostRevision < ActiveRecord::Base
   serialize :modifications, Hash
 
   def body_changes
-    changes_for("cooked", "raw")
+    cooked_diff = DiscourseDiff.new(previous("cooked"), current("cooked"))
+    raw_diff = DiscourseDiff.new(previous("raw"), current("raw"))
+
+    {
+      inline: cooked_diff.inline_html,
+      side_by_side: cooked_diff.side_by_side_html,
+      side_by_side_markdown: raw_diff.side_by_side_markdown
+    }
   end
 
   def category_changes
+    prev = previous("category_id")
+    cur = current("category_id")
+    return if prev == cur
+
     {
-      previous_category_id: previous("category_id"),
-      current_category_id: current("category_id"),
+      previous_category_id: prev,
+      current_category_id: cur,
     }
   end
 
   def title_changes
-    changes_for("title", nil, true)
-  end
-
-  def changes_for(name, markdown=nil, wrap=false)
-    prev = previous(name)
-    cur = current(name)
-
-    if wrap
-      prev = "<div>#{CGI::escapeHTML(prev)}</div>"
-      cur = "<div>#{CGI::escapeHTML(cur)}</div>"
-    end
+    prev = "<div>#{CGI::escapeHTML(previous("title"))}</div>"
+    cur = "<div>#{CGI::escapeHTML(current("title"))}</div>"
+    return if prev == cur
 
     diff = DiscourseDiff.new(prev, cur)
 
-    result = {
+    {
       inline: diff.inline_html,
       side_by_side: diff.side_by_side_html
     }
-
-    if markdown
-      diff = DiscourseDiff.new(previous(markdown), current(markdown))
-      result[:side_by_side_markdown] = diff.side_by_side_markdown
-    end
-
-    result
   end
 
   def previous(field)
@@ -54,12 +50,9 @@ class PostRevision < ActiveRecord::Base
   end
 
   def previous_revisions
-    @previous_revs ||=
-      PostRevision.where("post_id = ? AND number < ?",
-                              post_id,        number
-                      )
-                .order("number desc")
-                .to_a
+    @previous_revs ||= PostRevision.where("post_id = ? AND number < ?", post_id, number)
+                                   .order("number desc")
+                                   .to_a
   end
 
   def has_topic_data?