More refactoring to support extensibility of history

app/assets/javascripts/discourse/components/disabled-icon.js.es6

@@ -0,0 +1,4 @@
+export default Ember.Component.extend({
+  tagName: 'span',
+  classNameBindings: [':fa-stack'],
+});

app/assets/javascripts/discourse/controllers/history.js.es6

@@ -81,23 +81,14 @@ export default ObjectController.extend(ModalFunctionality, {
     }
   }.property("category_id_changes"),
 
-  wikiDiff: function() {
+  wikiDisabled: function() {
     var changes = this.get("wiki_changes");
-    if (changes) {
-      return changes["current"] ?
-             '<span class="fa-stack"><i class="fa fa-pencil-square-o fa-stack-2x"></i></span>' :
-             '<span class="fa-stack"><i class="fa fa-pencil-square-o fa-stack-2x"></i><i class="fa fa-ban fa-stack-2x"></i></span>';
-    }
-  }.property("wiki_changes"),
+    return changes && !changes['current'];
+  }.property('wiki_changes'),
 
-  postTypeDiff: function () {
-    var moderator = Discourse.Site.currentProp('post_types.moderator_action');
+  postTypeDisabled: function () {
     var changes = this.get("post_type_changes");
-    if (changes) {
-      return changes["current"] === moderator ?
-             '<span class="fa-stack"><i class="fa fa-shield fa-stack-2x"></i></span>' :
-             '<span class="fa-stack"><i class="fa fa-shield fa-stack-2x"></i><i class="fa fa-ban fa-stack-2x"></i></span>';
-    }
+    return (changes && changes['current'] !== this.site.get('post_types.moderator_action'));
   }.property("post_type_changes"),
 
   titleDiff: function() {

app/assets/javascripts/discourse/helpers/fa-icon.js.es6

@@ -1,5 +1,13 @@
-export function iconHTML(icon, label) {
-  var html = "<i class='fa fa-" + icon + "'";
+import registerUnbound from 'discourse/helpers/register-unbound';
+
+export function iconClasses(icon, modifier) {
+  var classes = "fa fa-" + icon;
+  if (modifier) { classes += " fa-" + modifier; }
+  return classes;
+}
+
+export function iconHTML(icon, label, modifier) {
+  var html = "<i class='" + iconClasses(icon, modifier) + "'";
   if (label) { html += " aria-hidden='true'"; }
   html += "></i>";
   if (label) {
@@ -8,9 +16,7 @@ export function iconHTML(icon, label) {
   return html;
 }
 
-Handlebars.registerHelper('fa-icon', function(icon, options) {
-  var label;
-  if (options.hash) { label = options.hash.label; }
 
-  return new Handlebars.SafeString(iconHTML(icon, label));
+registerUnbound('fa-icon', function(icon, params) {
+  return new Handlebars.SafeString(iconHTML(icon, params.label, params.modifier));
 });

app/assets/javascripts/discourse/models/_post.js

@@ -470,7 +470,7 @@ Discourse.Post.reopenClass({
 
   loadRevision: function(postId, version) {
     return Discourse.ajax("/posts/" + postId + "/revisions/" + version + ".json").then(function (result) {
-      return Em.Object.create(result);
+      return Ember.Object.create(result);
     });
   },
 

app/assets/javascripts/discourse/templates/components/disabled-icon.hbs

@@ -0,0 +1,5 @@
+{{fa-icon icon modifier="stack-2x"}}
+
+{{#if disabled}}
+  {{fa-icon "ban" modifier="stack-2x"}}
+{{/if}}

app/assets/javascripts/discourse/templates/modal/history.hbs

@@ -40,10 +40,10 @@
         → {{bound-avatar-template user_changes.current.avatar_template "small"}} {{user_changes.current.username}}
       {{/if}}
       {{#if wiki_changes}}
-        — {{{wikiDiff}}}
+        — {{disabled-icon icon="pencil-square-o" secondary=wikiDisabled}}
       {{/if}}
       {{#if post_type_changes}}
-        — {{{postTypeDiff}}}
+        — {{disabled-icon icon="shield" disabled=postTypeDisabled}}
       {{/if}}
       {{#if category_id_changes}}
         — {{{previousCategory}}} → {{{currentCategory}}}
@@ -65,12 +65,12 @@
       {{/if}}
       {{#if wiki_changes}}
         <div class="row">
-          {{{wikiDiff}}}
+          {{disabled-icon icon="pencil-square-o" secondary=wikiDisabled}}
         </div>
       {{/if}}
       {{#if post_type_changes}}
         <div class="row">
-          {{{postTypeDiff}}}
+          {{disabled-icon icon="shield" disabled=postTypeDisabled}}
         </div>
       {{/if}}
       {{#if category_id_changes}}
@@ -79,6 +79,9 @@
         </div>
       {{/if}}
     {{/if}}
+
+    {{plugin-outlet "post-revisions"}}
+
     <div class="row">
       {{{bodyDiff}}}
     </div>

app/controllers/posts_controller.rb

@@ -414,7 +414,7 @@ class PostsController < ApplicationController
       result[:is_warning] = (params[:is_warning] == "true")
     end
 
-    PostRevisor.tracked_fields.keys.each do |f|
+    PostRevisor.tracked_topic_fields.keys.each do |f|
       params.permit(f => [])
       result[f] = params[f] if params.has_key?(f)
     end

app/controllers/topics_controller.rb

@@ -126,7 +126,7 @@ class TopicsController < ApplicationController
     guardian.ensure_can_edit!(topic)
 
     changes = {}
-    PostRevisor.tracked_fields.keys.each do |f|
+    PostRevisor.tracked_topic_fields.keys.each do |f|
       changes[f] = params[f] if params.has_key?(f)
     end
 

app/serializers/post_revision_serializer.rb

@@ -40,9 +40,7 @@ class PostRevisionSerializer < ApplicationSerializer
     end
   end
 
-  add_compared_field :category_id
   add_compared_field :wiki
-  add_compared_field :post_type
 
   def previous_hidden
     previous["hidden"]
@@ -167,19 +165,27 @@ class PostRevisionSerializer < ApplicationSerializer
       return @all_revisions if @all_revisions
 
       post_revisions = PostRevision.where(post_id: object.post_id).order(:number).to_a
-      post_revisions << PostRevision.new(
-        number: post_revisions.last.number + 1,
-        hidden: post.hidden,
-        modifications: {
+
+      latest_modifications = {
         "raw" => [post.raw],
         "cooked" => [post.cooked],
         "edit_reason" => [post.edit_reason],
         "wiki" => [post.wiki],
         "post_type" => [post.post_type],
-          "user_id" => [post.user_id],
-          "title" => [topic.title],
-          "category_id" => [topic.category_id],
+        "user_id" => [post.user_id]
       }
+
+      # For the topic fields, let's get the values from a serializer
+      PostRevisor.tracked_topic_fields.keys.each do |field|
+        if topic.respond_to?(field)
+          latest_modifications[field.to_s] = [topic.send(field)]
+        end
+      end
+
+      post_revisions << PostRevision.new(
+        number: post_revisions.last.number + 1,
+        hidden: post.hidden,
+        modifications: latest_modifications
       )
 
       @all_revisions = []

app/serializers/topic_view_serializer.rb

@@ -3,9 +3,16 @@ require_dependency 'pinned_check'
 class TopicViewSerializer < ApplicationSerializer
   include PostStreamSerializerMixin
 
-  # These attributes will be delegated to the topic
-  def self.topic_attributes
-    [:id,
+  def self.attributes_from_topic(*list)
+    [list].flatten.each do |attribute|
+      attributes(attribute)
+      class_eval %{def #{attribute}
+        object.topic.#{attribute}
+      end}
+    end
+  end
+
+  attributes_from_topic :id,
                         :title,
                         :fancy_title,
                         :posts_count,
@@ -23,8 +30,7 @@ class TopicViewSerializer < ApplicationSerializer
                         :slug,
                         :category_id,
                         :word_count,
-     :deleted_at]
-  end
+                        :deleted_at
 
   attributes :draft,
              :draft_key,
@@ -45,14 +51,6 @@ class TopicViewSerializer < ApplicationSerializer
              :chunk_size,
              :bookmarked
 
-  # Define a delegator for each attribute of the topic we want
-  attributes(*topic_attributes)
-  topic_attributes.each do |ta|
-    class_eval %{def #{ta}
-      object.topic.#{ta}
-    end}
-  end
-
   # TODO: Split off into proper object / serializer
   def details
     result = {

lib/post_revisor.rb

@@ -4,7 +4,7 @@ class PostRevisor
 
   # Helps us track changes to a topic.
   #
-  # It's passed to `Topic.track_field` callbacks so they can record if they
+  # It's passed to `track_topic_fields` callbacks so they can record if they
   # changed a value or not. This is needed for things like custom fields.
   class TopicChanges
     attr_reader :topic, :user
@@ -47,13 +47,29 @@ class PostRevisor
     @topic = topic || post.topic
   end
 
-  def self.tracked_fields
-    @@tracked_fields ||= {}
-    @@tracked_fields
+  def self.tracked_topic_fields
+    @@tracked_topic_fields ||= {}
+    @@tracked_topic_fields
   end
 
-  def self.track_field(field, &block)
-    tracked_fields[field] = block
+  def self.track_topic_field(field, &block)
+    tracked_topic_fields[field] = block
+
+    # Define it in the serializer unless it already has been defined
+    unless PostRevisionSerializer.instance_methods(false).include?("#{field}_changes".to_sym)
+      PostRevisionSerializer.add_compared_field(field)
+    end
+  end
+
+  # Fields we want to record revisions for by default
+  track_topic_field(:title) do |tc, title|
+    tc.record_change('title', tc.topic.title, title)
+    tc.topic.title = title
+  end
+
+  track_topic_field(:category_id) do |tc, category_id|
+    tc.record_change('category_id', tc.topic.category_id, category_id)
+    tc.check_result(tc.topic.change_category_to_id(category_id))
   end
 
   # AVAILABLE OPTIONS:
@@ -139,7 +155,7 @@ class PostRevisor
   end
 
   def topic_changed?
-    PostRevisor.tracked_fields.keys.any? {|f| @fields.has_key?(f)}
+    PostRevisor.tracked_topic_fields.keys.any? {|f| @fields.has_key?(f)}
   end
 
   def revise_post
@@ -217,7 +233,7 @@ class PostRevisor
 
   def update_topic
     Topic.transaction do
-      PostRevisor.tracked_fields.each do |f, cb|
+      PostRevisor.tracked_topic_fields.each do |f, cb|
         if !@topic_changes.errored? && @fields.has_key?(f)
           cb.call(@topic_changes, @fields[f])
         end
@@ -360,13 +376,3 @@ class PostRevisor
 
 end
 
-# Fields we want to record revisions for by default
-PostRevisor.track_field(:title) do |tc, title|
-  tc.record_change('title', tc.topic.title, title)
-  tc.topic.title = title
-end
-
-PostRevisor.track_field(:category_id) do |tc, category_id|
-  tc.record_change('category_id', tc.topic.category_id, category_id)
-  tc.check_result(tc.topic.change_category_to_id(category_id))
-end