better revision history

Gemfile

@@ -69,8 +69,6 @@ gem 'ember-source', '~> 1.2.0.1'
 gem 'handlebars-source', '~> 1.1.2'
 gem 'barber'
 
-gem 'vestal_versions', git: 'https://github.com/SamSaffron/vestal_versions'
-
 gem 'message_bus'
 gem 'rails_multisite', path: 'vendor/gems/rails_multisite'
 gem 'simple_handlebars_rails', path: 'vendor/gems/simple_handlebars_rails'
@@ -124,7 +122,6 @@ gem 'slim'  # required for sidekiq-web
 # URGENT fix needed see: https://github.com/cowboyd/therubyracer/pull/280
 gem 'therubyracer', require: 'v8', git: 'https://github.com/SamSaffron/therubyracer.git'
 gem 'thin', require: false
-gem 'diffy', '>= 3.0', require: false
 gem 'highline', require: false
 gem 'rack-protection' # security
 

Gemfile_rails4.lock

@@ -31,14 +31,6 @@ GIT
       libv8 (~> 3.16.14.0)
       ref
 
-GIT
-  remote: https://github.com/SamSaffron/vestal_versions
-  revision: 007b30a5274db7db55da745a4482243559247782
-  specs:
-    vestal_versions (1.2.3)
-      activerecord (> 3.0)
-      activesupport (> 3.0)
-
 GIT
   remote: https://github.com/callahad/omniauth-browserid.git
   revision: af62d667626c1622de6fe13b60849c3640765ab1
@@ -129,7 +121,6 @@ GEM
     daemons (1.1.9)
     debug_inspector (0.0.2)
     diff-lcs (1.2.4)
-    diffy (3.0.1)
     ember-data-source (0.14)
       ember-source
     ember-rails (0.14.1)
@@ -451,7 +442,6 @@ DEPENDENCIES
   better_errors
   binding_of_caller
   certified
-  diffy (>= 3.0)
   discourse_plugin!
   email_reply_parser!
   ember-rails
@@ -532,4 +522,3 @@ DEPENDENCIES
   uglifier
   unf
   unicorn
-  vestal_versions!

app/assets/javascripts/discourse/controllers/history_controller.js

@@ -1,6 +1,3 @@
-/*jshint newcap:false*/
-/*global diff_match_patch:true assetPath:true*/
-
 /**
   This controller handles displaying of history
 
@@ -11,79 +8,43 @@
   @module Discourse
 **/
 Discourse.HistoryController = Discourse.ObjectController.extend(Discourse.ModalFunctionality, {
-  diffLibraryLoaded: false,
-  diff: null,
-
-  init: function(){
-    this._super();
-    var historyController = this;
-    $LAB.script(assetPath('defer/google_diff_match_patch')).wait(function(){
-      historyController.set('diffLibraryLoaded', true);
-    });
-  },
-
-  loadSide: function(side) {
-    if (this.get("version" + side)) {
-      var orig = this.get('model');
-      var version = this.get("version" + side + ".number");
-      if (version === orig.get('version')) {
-        this.set("post" + side, orig);
-      } else {
-        var historyController = this;
-        Discourse.Post.loadVersion(orig.get('id'), version).then(function(post) {
-          historyController.set("post" + side, post);
-        });
-      }
-    }
-  },
-
-  changedLeftVersion: function() {
-    this.loadSide("Left");
-  }.observes('versionLeft'),
-
-  changedRightVersion: function() {
-    this.loadSide("Right");
-  }.observes('versionRight'),
-
-  loadedPosts: function() {
-    if (this.get('diffLibraryLoaded') && this.get('postLeft') && this.get('postRight')) {
-      var dmp = new diff_match_patch(),
-          before = this.get("postLeft.cooked"),
-          after = this.get("postRight.cooked"),
-          diff = dmp.diff_main(before, after);
-      dmp.diff_cleanupSemantic(diff);
-      this.set('diff', dmp.diff_prettyHtml(diff));
-    }
-  }.observes('diffLibraryLoaded', 'postLeft', 'postRight'),
-
-  refresh: function() {
-    this.setProperties({
-      loading: true,
-      postLeft: null,
-      postRight: null
-    });
-
-    var historyController = this;
-    this.get('model').loadVersions().then(function(result) {
-      _.each(result,function(item) {
-
-        var age = Discourse.Formatter.relativeAge(new Date(item.created_at), {
-          format: 'medium',
-          leaveAgo: true,
-          wrapInSpan: false});
-
-        item.description = "v" + item.number + " - " + age + " - " + I18n.t("changed_by", { author: item.display_username });
-      });
-
-      historyController.setProperties({
   loading: false,
-        versionLeft: result[0],
+  viewMode: "side_by_side",
-        versionRight: result[result.length-1],
+
-        versions: result
+  refresh: function(postId, postVersion) {
+    this.set("loading", true);
+
+    var self = this;
+    Discourse.Post.loadRevision(postId, postVersion).then(function (result) {
+      self.setProperties({
+        loading: false,
+        model: result
       });
     });
+  },
+
+  createdAtDate: function() { return moment(this.get("created_at")).format("LLLL"); }.property("created_at"),
+
+  previousVersionNumber: function() { return this.get("version") - 1; }.property("version"),
+  currentVersionNumber: Em.computed.alias("version"),
+
+  isFirstVersion: Em.computed.equal("version", 2),
+  isLastVersion: Discourse.computed.propertyEqual("version", "revisions_count"),
+
+  displayingInline: Em.computed.equal("viewMode", "inline"),
+  displayingSideBySide: Em.computed.equal("viewMode", "side_by_side"),
+  displayingSideBySideMarkdown: Em.computed.equal("viewMode", "side_by_side_markdown"),
+
+  diff: function() { return this.get(this.get("viewMode")); }.property("inline", "side_by_side", "side_by_side_markdown", "viewMode"),
+
+  actions: {
+    loadFirstVersion: function() { this.refresh(this.get("post_id"), 2); },
+    loadPreviousVersion: function() { this.refresh(this.get("post_id"), this.get("version") - 1); },
+    loadNextVersion: function() { this.refresh(this.get("post_id"), this.get("version") + 1); },
+    loadLastVersion: function() { this.refresh(this.get("post_id"), this.get("revisions_count")); },
+
+    displayInline: function() { this.set("viewMode", "inline"); },
+    displaySideBySide: function() { this.set("viewMode", "side_by_side"); },
+    displaySideBySideMarkdown: function() { this.set("viewMode", "side_by_side_markdown"); }
   }
-
 });
-
-

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

@@ -337,10 +337,6 @@ Discourse.Post = Discourse.Model.extend({
     });
   },
 
-  loadVersions: function() {
-    return Discourse.ajax("/posts/" + (this.get('id')) + "/versions.json");
-  },
-
   // Whether to show replies directly below
   showRepliesBelow: function() {
     var reply_count = this.get('reply_count');
@@ -403,8 +399,8 @@ Discourse.Post.reopenClass({
     });
   },
 
-  loadVersion: function(postId, version, callback) {
+  loadRevision: function(postId, version) {
-    return Discourse.ajax("/posts/" + postId + ".json?version=" + version).then(function(result) {
+    return Discourse.ajax("/posts/" + postId + "/revisions/" + version + ".json").then(function (result) {
       return Discourse.Post.create(result);
     });
   },

app/assets/javascripts/discourse/routes/topic_route.js

@@ -49,7 +49,7 @@ Discourse.TopicRoute = Discourse.Route.extend({
 
     showHistory: function(post) {
       Discourse.Route.showModal(this, 'history', post);
-      this.controllerFor('history').refresh();
+      this.controllerFor('history').refresh(post.get("id"), post.get("version"));
       this.controllerFor('modal').set('modalClass', 'history-modal');
     },
 

app/assets/javascripts/discourse/templates/modal/history.js.handlebars

@@ -1,48 +1,24 @@
 <div class="modal-body">
-
   {{#if loading}}
     {{i18n loading}}
   {{else}}
-    {{#if versions}}
+    <div>
-      <div class='span8'>
+      <div id="revision-controls">
-
+        <button class="btn standard" title="{{i18n post.revisions.controls.first}}" {{action loadFirstVersion}} {{bindAttr disabled=isFirstVersion}}><i class="fa fa-fast-backward"></i></button>
-        {{view Ember.Select
+        <button class="btn standard" title="{{i18n post.revisions.controls.previous}}" {{action loadPreviousVersion}} {{bindAttr disabled=isFirstVersion}}><i class="fa fa-backward"></i></button>
-          contentBinding="versions"
+        {{{i18n post.revisions.controls.comparing_previous_to_current_out_of_total previous=previousVersionNumber current=currentVersionNumber total=revisions_count}}}
-          optionLabelPath="content.description"
+        <button class="btn standard" title="{{i18n post.revisions.controls.next}}" {{action loadNextVersion}} {{bindAttr disabled=isLastVersion}}><i class="fa fa-forward"></i></button>
-          optionValuePath="content.number"
+        <button class="btn standard" title="{{i18n post.revisions.controls.last}}" {{action loadLastVersion}} {{bindAttr disabled=isLastVersion}}><i class="fa fa-fast-forward"></i></button>
-          selectionBinding="versionLeft"}}
-
-        <div class='contents'>
-          {{#if postLeft}}
-            {{{postLeft.cooked}}}
-          {{else}}
-            <div class='history-loading'>{{i18n loading}}</div>
-          {{/if}}
       </div>
-
+      <div id="display-modes">
+        <button {{bindAttr class=":btn displayingInline:btn-primary:standard"}} title="{{i18n post.revisions.displays.inline.title}}" {{action displayInline}}>{{{i18n post.revisions.displays.inline.button}}}</button>
+        <button {{bindAttr class=":btn displayingSideBySide:btn-primary:standard"}} title="{{i18n post.revisions.displays.side_by_side.title}}" {{action displaySideBySide}}>{{{i18n post.revisions.displays.side_by_side.button}}}</button>
+        <button {{bindAttr class=":btn displayingSideBySideMarkdown:btn-primary:standard"}} title="{{i18n post.revisions.displays.side_by_side_markdown.title}}" {{action displaySideBySideMarkdown}}>{{{i18n post.revisions.displays.side_by_side_markdown.button}}}</button>
+      </div>
+    </div>
+    <div id="revision-details">
+      {{i18n post.revisions.details.edited_by}} {{avatar this imageSize="small"}} {{username}} <span class="date">{{date path="created_at" leaveAgo="true"}}</span> {{#if edit_reason}} &mdash; <span class="edit-reason">{{edit_reason}}</span>{{/if}}
     </div>
-
-      <div class='span8 offset1'>
-        {{view Ember.Select
-          contentBinding="versions"
-          optionLabelPath="content.description"
-          optionValuePath="content.number"
-          selectionBinding="versionRight"}}
-
-        {{#if postRight.edit_reason}}
-          <p><strong>{{i18n post.edit_reason}}</strong>{{postRight.edit_reason}}</p>
-        {{/if}}
-
-        <div class='contents'>
-          {{#if diff}}
     {{{diff}}}
-          {{else}}
-            <div class='history-loading'>{{i18n loading}}</div>
   {{/if}}
 </div>
-
-      </div>
-    {{/if}}
-  {{/if}}
-
-</div>

app/assets/javascripts/discourse/templates/user/stream.js.handlebars

@@ -16,6 +16,7 @@
         <i class="icon {{unbound icon}}"></i>
         {{#groupedEach items}}
           <a href="{{unbound userUrl}}" class='avatar-link'><div class='avatar-wrapper'>{{avatar this imageSize="tiny" extraClasses="actor" ignoreTitle="true"}}</div></a>
+          {{#if edit_reason}} &mdash; <span class="edit-reason">{{unbound edit_reason}}</span>{{/if}}
         {{/groupedEach}}
       </div>
     {{/groupedEach}}

app/assets/javascripts/discourse/views/modal/history_view.js

@@ -8,5 +8,11 @@
 **/
 Discourse.HistoryView = Discourse.ModalBodyView.extend({
   templateName: 'modal/history',
-  title: I18n.t('history')
+  title: I18n.t('history'),
+
+  resizeModal: function(){
+    var viewPortHeight = $(window).height();
+    this.$(".modal-body").css("max-height", Math.floor(0.8 * viewPortHeight) + "px");
+  }.on("didInsertElement")
+
 });

app/assets/javascripts/discourse/views/modal/modal_body_view.js

@@ -10,14 +10,19 @@ Discourse.ModalBodyView = Discourse.View.extend({
 
   // Focus on first element
   didInsertElement: function() {
+    var self = this;
+
     $('#discourse-modal').modal('show');
 
+    $('#discourse-modal').one("hide", function () {
+      self.get("controller").send("closeModal");
+    });
+
     $('#modal-alert').hide();
 
     if (!Discourse.Mobile.mobileView) {
-      var modalBodyView = this;
       Em.run.schedule('afterRender', function() {
-        modalBodyView.$('input:first').focus();
+        self.$('input:first').focus();
       });
     }
 

app/assets/stylesheets/desktop/history.scss

@@ -1,5 +1,4 @@
-// styles that apply to the popup that appears when you show the edit history
+// styles that apply to the popup that appears when you show the edit history of a post
-// of a post
 
 @import "common/foundation/variables";
 @import "common/foundation/mixins";
@@ -9,30 +8,105 @@
     min-width: 960px;
     min-height: 500px;
   }
-
+  #revision-controls {
+    float: left;
+  }
+  #display-modes {
+    text-align: right;
+  }
+  #revision-details {
+    background-color: #eee;
+    padding: 5px;
+    margin-top: 10px;
+  }
+  img {
+    max-width: 670px;
+    height: auto;
+  }
+  .inline-diff {
+    width: 670px;
+    word-wrap: break-word;
+  }
+  .markdown {
+    word-wrap: break-word;
+    white-space: pre-wrap;
+    font-family: monospace;
+    font-size: 12px;
+    width: 100%;
+    border-collapse: collapse;
+    border-spacing: 0px;
+    td {
+      width: 50%;
+      vertical-align: top;
+    }
+  }
+  .span8, .markdown {
+    img {
+      max-width: 400px;
+    }
+  }
+  ins, .diff-ins {
+    code, img {
+      border: 2px solid #405A04;
+    }
+    img {
+      opacity: .75;
+      filter: alpha(opacity=75);
+    }
+    a {
+      color: #2D4003;
+      text-decoration: none;
+    }
+  }
+  img.diff-ins, code.diff-ins {
+    border: 2px solid #405A04;
+  }
+  img.diff-ins {
+    opacity: .75;
+    filter: alpha(opacity=75);
+  }
+  .diff-ins {
+    background: #f9ffe1;
+  }
   ins {
-    background: #e6ffe6;
+    color: #405A04;
+    background: #D1E1AD;
+  }
+  del, .diff-del {
+    code, img {
+      border: 2px solid #A82400;
+    }
+    img {
+      opacity: .5;
+      filter: alpha(opacity=50);
+    }
+    a {
+      color: #400E00;
+      text-decoration: none;
+    }
+  }
+  img.diff-del, code.diff-del {
+    border: 2px solid #A82400;
+  }
+  img.diff-del {
+    opacity: .5;
+    filter: alpha(opacity=50);
+  }
+  .diff-del {
+    background: #fff4f4;
   }
   del {
-    background: #ffe6e6;
+    color: #A82400;
+    background: #E5BDB2;
+  }
+  span.date {
+    font-weight: bold;
+  }
+  span.edit-reason {
+    background-color: #ffffcc;
+    padding: 3px 5px 5px 5px;
   }
   .modal-header {
     height: 42px;
   }
-  .history-loading {
-    margin: 25px 0;
-    width: 120px;
-    font-size: 20px;
-    padding: 8px 0 30px 30px;
-    background: {
-      image: image-url("spinner_96.gif");
-      repeat: no-repeat;
-      size: 25px 25px;
-      position: 0 4px;
-    };
-  }
-  select {
-    height: auto;
-	width: auto;
-  }
 }

app/assets/stylesheets/desktop/user.scss

@@ -301,6 +301,10 @@
       margin-bottom: 4px;
       font-size: 14px;
     }
+    .edit-reason {
+      background-color: #ffffcc;
+      padding: 3px 5px 5px 5px;
+    }
   }
 }
 

app/assets/stylesheets/mobile/history.scss

@@ -21,26 +21,6 @@
     padding-bottom: 10px;
   }
   .modal-body {padding-top: 0px;}
-  .history-loading {
-    margin: 25px 0;
-    width: 120px;
-    font-size: 20px;
-    padding: 8px 0 30px 30px;
-    background: {
-      image: image-url("spinner_96.gif");
-      repeat: no-repeat;
-      size: 25px 25px;
-      position: 0 4px;
-    };
-  }
-  select {
-    height: auto;
-	width: auto;
-  font-size: 16px;
-  position: fixed;
-  margin-top: -45px;
-  background-color: #fff;
-  }
 }
 
 .offset1 {display: none;}

app/assets/stylesheets/mobile/user.scss

@@ -232,6 +232,10 @@
       margin-bottom: 4px;
       font-size: 14px;
     }
+    .edit-reason {
+      background-color: #ffffcc;
+      padding: 3px 5px 5px 5px;
+    }
   }
 }
 

app/controllers/posts_controller.rb

@@ -5,7 +5,7 @@ require_dependency 'distributed_memoizer'
 class PostsController < ApplicationController
 
   # Need to be logged in for all actions here
-  before_filter :ensure_logged_in, except: [:show, :replies, :by_number, :short_link, :versions, :reply_history]
+  before_filter :ensure_logged_in, except: [:show, :replies, :by_number, :short_link, :reply_history, :revisions]
 
   skip_before_filter :store_incoming_links, only: [:short_link]
   skip_before_filter :check_xhr, only: [:markdown,:short_link]
@@ -67,6 +67,7 @@ class PostsController < ApplicationController
     #  we should allow for title changes and category changes here
     #  we should also move all of this to a post updater.
     if post.post_number == 1 && (params[:title] || params[:post][:category])
+      post.topic.acting_user = current_user
       post.topic.title = params[:title] if params[:title]
       Topic.transaction do
         post.topic.change_category(params[:post][:category])
@@ -84,7 +85,6 @@ class PostsController < ApplicationController
       TopicLink.extract_from(post)
     end
 
-
     if post.errors.present?
       render_json_error(post)
       return
@@ -104,6 +104,12 @@ class PostsController < ApplicationController
     render_json_dump(result)
   end
 
+  def show
+    @post = find_post_from_params
+    @post.revert_to(params[:version].to_i) if params[:version].present?
+    render_post_json(@post)
+  end
+
   def by_number
     @post = Post.where(topic_id: params[:topic_id], post_number: params[:post_number]).first
     guardian.ensure_can_see!(@post)
@@ -114,16 +120,9 @@ class PostsController < ApplicationController
   def reply_history
     @post = Post.where(id: params[:id]).first
     guardian.ensure_can_see!(@post)
-
     render_serialized(@post.reply_history, PostSerializer)
   end
 
-  def show
-    @post = find_post_from_params
-    @post.revert_to(params[:version].to_i) if params[:version].present?
-    render_post_json(@post)
-  end
-
   def destroy
     post = find_post_from_params
     guardian.ensure_can_delete!(post)
@@ -161,18 +160,18 @@ class PostsController < ApplicationController
     render nothing: true
   end
 
-  # Retrieves a list of versions and who made them for a post
-  def versions
-    post = find_post_from_params
-    render_serialized(post.all_versions, VersionSerializer)
-  end
-
   # Direct replies to this post
   def replies
     post = find_post_from_params
     render_serialized(post.replies, PostSerializer)
   end
 
+  def revisions
+    post_revision = find_post_revision_from_params
+    post_revision_serializer = PostRevisionSerializer.new(post_revision, scope: guardian, root: false)
+    render_json_dump(post_revision_serializer)
+  end
+
   def bookmark
     post = find_post_from_params
     if current_user
@@ -185,12 +184,10 @@ class PostsController < ApplicationController
     render nothing: true
   end
 
-
   protected
 
   def find_post_from_params
     finder = Post.where(id: params[:id] || params[:post_id])
-
     # Include deleted posts if the user is staff
     finder = finder.with_deleted if current_user.try(:staff?)
 
@@ -199,6 +196,16 @@ class PostsController < ApplicationController
     post
   end
 
+  def find_post_revision_from_params
+    post_id = params[:id] || params[:post_id]
+    revision = params[:revision].to_i
+    raise Discourse::InvalidParameters.new(:revision) if revision < 2
+
+    post_revision = PostRevision.where(post_id: post_id, number: revision).first
+    guardian.ensure_can_see!(post_revision)
+    post_revision
+  end
+
   def render_post_json(post)
     post_serializer = PostSerializer.new(post, scope: guardian, root: false)
     post_serializer.add_raw = true
@@ -245,4 +252,5 @@ class PostsController < ApplicationController
         whitelisted[:meta_data] = params[:meta_data]
     end
   end
+
 end

app/controllers/topics_controller.rb

@@ -101,13 +101,14 @@ class TopicsController < ApplicationController
     # TODO: we may need smarter rules about converting archetypes
     topic.archetype = "regular" if current_user.admin? && archetype == 'regular'
 
+    topic.acting_user = current_user
+
     success = false
     Topic.transaction do
-      success = topic.save
+      success = topic.save && topic.change_category(params[:category])
-      success = topic.change_category(params[:category]) if success
     end
-    # this is used to return the title to the client as it may have been
+
-    # changed by "TextCleaner"
+    # this is used to return the title to the client as it may have been changed by "TextCleaner"
     success ? render_serialized(topic, BasicTopicSerializer) : render_json_error(topic)
   end
 

app/models/post.rb

@@ -15,8 +15,6 @@ class Post < ActiveRecord::Base
   include RateLimiter::OnCreateRecord
   include Trashable
 
-  versioned if: :raw_changed?
-
   rate_limit
   rate_limit :limit_posts_per_day
 
@@ -36,6 +34,9 @@ class Post < ActiveRecord::Base
 
   has_many :post_details
 
+  has_many :post_revisions
+  has_many :revisions, foreign_key: :post_id, class_name: 'PostRevision'
+
   validates_with ::Validators::PostValidator
 
   # We can pass several creating options to a post via attributes
@@ -317,12 +318,19 @@ class Post < ActiveRecord::Base
     self.cooked = cook(raw, topic_id: topic_id) unless new_record?
   end
 
+  after_save do
+    save_revision if self.version_changed?
+  end
+
+  after_update do
+    update_revision if self.changed?
+  end
+
   def advance_draft_sequence
     return if topic.blank? # could be deleted
     DraftSequence.next!(last_editor_id, topic.draft_key)
   end
 
-
   # TODO: move to post-analyzer?
   # Determine what posts are quoted by this post
   def extract_quoted_post_numbers
@@ -386,10 +394,17 @@ class Post < ActiveRecord::Base
     Post.where(id: post_ids).includes(:user, :topic).order(:id).to_a
   end
 
+  def revert_to(number)
+    return if number >= version
+    post_revision = PostRevision.where(post_id: id, number: number + 1).first
+    post_revision.modifications.each do |attribute, change|
+      attribute = "version" if attribute == "cached_version"
+      write_attribute(attribute, change[0])
+    end
+  end
+
   private
 
-
-
   def parse_quote_into_arguments(quote)
     return {} unless quote.present?
     args = {}
@@ -412,6 +427,27 @@ class Post < ActiveRecord::Base
       Post.where(id: post.id).update_all ['reply_count = reply_count + 1']
     end
   end
+
+  def save_revision
+    modifications = changes.extract!(:raw, :cooked, :edit_reason)
+    # make sure cooked is always present (oneboxes might not change the cooked post)
+    modifications["cooked"] = [self.cooked, self.cooked] unless modifications["cooked"].present?
+    PostRevision.create!(
+      user_id: last_editor_id,
+      post_id: id,
+      number: version,
+      modifications: modifications
+    )
+  end
+
+  def update_revision
+    revision = PostRevision.where(post_id: id, number: version).first
+    return unless revision
+    revision.user_id = last_editor_id
+    revision.modifications = changes.extract!(:raw, :cooked, :edit_reason)
+    revision.save
+  end
+
 end
 
 # == Schema Information
@@ -427,7 +463,7 @@ end
 #  created_at              :datetime         not null
 #  updated_at              :datetime         not null
 #  reply_to_post_number    :integer
-#  cached_version          :integer          default(1), not null
+#  version                 :integer          default(1), not null
 #  reply_count             :integer          default(0), not null
 #  quote_count             :integer          default(0), not null
 #  deleted_at              :datetime

app/models/post_alert_observer.rb

@@ -1,5 +1,5 @@
 class PostAlertObserver < ActiveRecord::Observer
-  observe :post, VestalVersions::Version, :post_action
+  observe :post, :post_action, :post_revision
 
   # Dispatch to an after_save_#{class_name} method
   def after_save(model)
@@ -46,15 +46,14 @@ class PostAlertObserver < ActiveRecord::Observer
                         post_action_id: post_action.id)
   end
 
-  def after_create_version(version)
+  def after_create_post_revision(post_revision)
-    post = version.versioned
+    post = post_revision.post
 
-    return unless post.is_a?(Post)
+    return if post_revision.user.blank?
-    return if version.user.blank?
+    return if post_revision.user_id == post.user_id
-    return if version.user_id == post.user_id
     return if post.topic.private_message?
 
-    create_notification(post.user, Notification.types[:edited], post, display_username: version.user.username)
+    create_notification(post.user, Notification.types[:edited], post, display_username: post_revision.user.username)
   end
 
   def after_create_post(post)

app/models/post_revision.rb

@@ -0,0 +1,6 @@
+class PostRevision < ActiveRecord::Base
+  belongs_to :post
+  belongs_to :user
+
+  serialize :modifications, Hash
+end

app/models/topic.rb

@@ -26,8 +26,6 @@ class Topic < ActiveRecord::Base
     2**31 - 1
   end
 
-  versioned if: :new_version_required?
-
   def featured_users
     @featured_users ||= TopicFeaturedUsers.new(self)
   end
@@ -97,6 +95,9 @@ class Topic < ActiveRecord::Base
   has_many :topic_invites
   has_many :invites, through: :topic_invites, source: :invite
 
+  has_many :topic_revisions
+  has_many :revisions, foreign_key: :topic_id, class_name: 'TopicRevision'
+
   # When we want to temporarily attach some data to a forum topic (usually before serialization)
   attr_accessor :user_data
   attr_accessor :posters  # TODO: can replace with posters_summary once we remove old list code
@@ -177,6 +178,8 @@ class Topic < ActiveRecord::Base
   end
 
   after_save do
+    save_revision if should_create_new_version?
+
     return if skip_callbacks
 
     if auto_close_at and (auto_close_at_changed? or auto_close_user_id_changed?)
@@ -184,6 +187,19 @@ class Topic < ActiveRecord::Base
     end
   end
 
+  def save_revision
+    TopicRevision.create!(
+      user_id: acting_user.id,
+      topic_id: id,
+      number: TopicRevision.where(topic_id: id).count + 2,
+      modifications: changes.extract!(:category, :title)
+    )
+  end
+
+  def should_create_new_version?
+    !new_record? && (category_id_changed? || title_changed?)
+  end
+
   def self.top_viewed(max = 10)
     Topic.listable_topics.visible.secured.order('views desc').limit(max)
   end
@@ -659,6 +675,14 @@ class Topic < ActiveRecord::Base
     category && category.read_restricted
   end
 
+  def acting_user
+    @acting_user || user
+  end
+
+  def acting_user=(u)
+    @acting_user = u
+  end
+
   private
 
   def update_category_topic_count_by(num)

app/models/topic_revision.rb

@@ -0,0 +1,6 @@
+class TopicRevision < ActiveRecord::Base
+  belongs_to :topic
+  belongs_to :user
+
+  serialize :modifications, Hash
+end

app/models/user_action.rb

@@ -97,7 +97,8 @@ SELECT
   coalesce(p.cooked, p2.cooked) cooked,
   CASE WHEN coalesce(p.deleted_at, p2.deleted_at, t.deleted_at) IS NULL THEN false ELSE true END deleted,
   p.hidden,
-  p.post_type
+  p.post_type,
+  p.edit_reason
 FROM user_actions as a
 JOIN topics t on t.id = a.target_topic_id
 LEFT JOIN posts p on p.id = a.target_post_id

app/serializers/post_revision_serializer.rb

@@ -0,0 +1,71 @@
+require_dependency "discourse_diff"
+
+class PostRevisionSerializer < ApplicationSerializer
+  attributes :post_id,
+             :version,
+             :revisions_count,
+             :username,
+             :display_username,
+             :avatar_template,
+             :created_at,
+             :edit_reason,
+             :inline,
+             :side_by_side,
+             :side_by_side_markdown
+
+  def version
+    object.number
+  end
+
+  def revisions_count
+    object.post.version
+  end
+
+  def username
+    object.user.username_lower
+  end
+
+  def display_username
+    object.user.username
+  end
+
+  def avatar_template
+    object.user.avatar_template
+  end
+
+  def edit_reason
+    return unless object.modifications["edit_reason"].present?
+    object.modifications["edit_reason"][1]
+  end
+
+  def inline
+    DiscourseDiff.new(previous_cooked, cooked).inline_html
+  end
+
+  def side_by_side
+    DiscourseDiff.new(previous_cooked, cooked).side_by_side_html
+  end
+
+  def side_by_side_markdown
+    DiscourseDiff.new(previous_raw, raw).side_by_side_text
+  end
+
+  private
+
+  def previous_cooked
+    @previous_cooked ||= object.modifications["cooked"][0]
+  end
+
+  def previous_raw
+    @previous_raw ||= object.modifications["raw"][0]
+  end
+
+  def cooked
+    @cooked ||= object.modifications["cooked"][1]
+  end
+
+  def raw
+    @raw ||= object.modifications["raw"][1]
+  end
+
+end

app/serializers/post_serializer.rb

@@ -98,10 +98,6 @@ class PostSerializer < BasicPostSerializer
     object.score || 0
   end
 
-  def version
-    object.cached_version
-  end
-
   def user_title
     object.user.try(:title)
   end

app/serializers/user_action_serializer.rb

@@ -21,7 +21,8 @@ class UserActionSerializer < ApplicationSerializer
              :title,
              :deleted,
              :hidden,
-             :moderator_action
+             :moderator_action,
+             :edit_reason
 
   def excerpt
     PrettyText.excerpt(object.cooked,300) if object.cooked

app/serializers/version_serializer.rb

@@ -1,17 +0,0 @@
-class VersionSerializer < ApplicationSerializer
-
-  attributes :number, :display_username, :created_at
-
-  def number
-    object[:number]
-  end
-
-  def display_username
-    object[:display_username]
-  end
-
-  def created_at
-    object[:created_at]
-  end
-
-end

config/initializers/vestal_versions.rb

@@ -1,9 +0,0 @@
-VestalVersions.configure do |config|
-  # Place any global options here. For example, in order to specify your own version model to use
-  # throughout the application, simply specify:
-  #
-  # config.class_name = "MyCustomVersion"
-  #
-  # Any options passed to the "versioned" method in the model itself will override this global
-  # configuration.
-end

config/locales/client.en.yml

@@ -946,6 +946,26 @@ en:
           one: "Are you sure you want to delete that post?"
           other: "Are you sure you want to delete all those posts?"
 
+      revisions:
+        controls:
+          first: "First revision"
+          previous: "Previous revision"
+          next: "Next revision"
+          last: "Last revision"
+          comparing_previous_to_current_out_of_total: "<strong>#{{previous}}</strong> vs. <strong>#{{current}}</strong> (out of {{total}})"
+        displays:
+          inline:
+            title: "Show the rendered output with additions and removals inline"
+            button: '<i class="fa fa-square-o"></i> HTML'
+          side_by_side:
+            title: "Show the rendered output diffs side-by-side"
+            button: '<i class="fa fa-columns"></i> HTML'
+          side_by_side_markdown:
+            title: "Show the markdown source diffs side-by-side"
+            button: '<i class="fa fa-columns"></i> Markdown'
+        details:
+          edited_by: "Edited by"
+
     category:
       can: 'can&hellip; '
       none: '(no category)'

config/routes.rb

@@ -162,9 +162,9 @@ Discourse::Application.routes.draw do
   get 'posts/by_number/:topic_id/:post_number' => 'posts#by_number'
   get 'posts/:id/reply-history' => 'posts#reply_history'
   resources :posts do
-    get 'versions'
     put 'bookmark'
     get 'replies'
+    get 'revisions/:revision' => 'posts#revisions'
     put 'recover'
     collection do
       delete 'destroy_many'

db/migrate/20131209091702_create_post_revisions.rb

@@ -0,0 +1,25 @@
+class CreatePostRevisions < ActiveRecord::Migration
+  def up
+    create_table :post_revisions do |t|
+      t.belongs_to :user
+      t.belongs_to :post
+      t.text :modifications
+      t.integer :number
+      t.timestamps
+    end
+
+    execute "INSERT INTO post_revisions (user_id, post_id, modifications, number, created_at, updated_at)
+             SELECT user_id, versioned_id, modifications, number, created_at, updated_at
+             FROM   versions
+             WHERE  versioned_type = 'Post'"
+
+    change_table :post_revisions do |t|
+      t.index :post_id
+      t.index [:post_id, :number]
+    end
+  end
+
+  def down
+    drop_table :post_revisions
+  end
+end

db/migrate/20131209091742_create_topic_revisions.rb

@@ -0,0 +1,25 @@
+class CreateTopicRevisions < ActiveRecord::Migration
+  def up
+    create_table :topic_revisions do |t|
+      t.belongs_to :user
+      t.belongs_to :topic
+      t.text :modifications
+      t.integer :number
+      t.timestamps
+    end
+
+    execute "INSERT INTO topic_revisions (user_id, topic_id, modifications, number, created_at, updated_at)
+             SELECT user_id, versioned_id, modifications, number, created_at, updated_at
+             FROM   versions
+             WHERE  versioned_type = 'Topic'"
+
+    change_table :topic_revisions do |t|
+      t.index :topic_id
+      t.index [:topic_id, :number]
+    end
+  end
+
+  def down
+    drop_table :topic_revisions
+  end
+end

db/migrate/20131210234530_rename_version_column.rb

@@ -0,0 +1,9 @@
+class RenameVersionColumn < ActiveRecord::Migration
+
+  def change
+    add_column :posts, :version, :integer, default: 1, null: false
+    execute "UPDATE posts SET version = cached_version"
+    remove_column :posts, :cached_version
+  end
+
+end

lib/cooked_post_processor.rb

@@ -226,7 +226,7 @@ class CookedPostProcessor
     # have we enough disk space?
     return if disable_if_low_on_disk_space
     # we only want to run the job whenever it's changed by a user
-    return if @post.updated_by == Discourse.system_user
+    return if @post.last_editor_id == Discourse.system_user.id
     # make sure no other job is scheduled
     Jobs.cancel_scheduled_job(:pull_hotlinked_images, post_id: @post.id)
     # schedule the job

lib/diff_engine.rb

@@ -1,6 +1,4 @@
-require 'diffy'
+# This class is used to generate diffs, it will be consumed by the UI on the client that displays diffs.
-# This class is used to generate diffs, it will be consumed by the UI on
-# on the client the displays diffs.
 #
 # There are potential performance issues associated with diffing large amounts of completely
 # different text, see answer here for optimization if needed
@@ -8,18 +6,20 @@ require 'diffy'
 
 class DiffEngine
 
-  # generate an html friendly diff similar to the way Stack Exchange generates
+  # Generate an html friendly diff
-  # html diffs
   #
   #  returns: html containing decorations indicating the changes
   def self.html_diff(html_before, html_after)
-    Diffy::Diff.new(html_before, html_after, {allow_empty_diff: false}).to_s(:html)
+    # tokenize
+    # remove leading/trailing common
+    # SES
+    # format diff
   end
 
-  # same as html diff, except that it operates on markdown
+  # Same as html diff, except that it operates on markdown
   #
   # returns html containing decorated areas where diff happened
   def self.markdown_diff(markdown_before, markdown_after)
-    Diffy::Diff.new(markdown_before, markdown_after).to_s(:html)
+
   end
 end

lib/discourse_diff.rb

@@ -0,0 +1,265 @@
+require_dependency "onpdiff"
+
+class DiscourseDiff
+
+  MAX_DIFFERENCE = 200
+
+  def initialize(before, after)
+    @before = before
+    @after = after
+
+    @block_by_block_diff = ONPDiff.new(tokenize_html_blocks(@before), tokenize_html_blocks(@after)).diff
+    @line_by_line_diff = ONPDiff.new(tokenize_line(@before), tokenize_line(@after)).short_diff
+  end
+
+  def inline_html
+    i = 0
+    inline = []
+    while i < @block_by_block_diff.length
+      op_code = @block_by_block_diff[i][1]
+      if op_code == :common then inline << @block_by_block_diff[i][0]
+      else
+        if op_code == :delete
+          opposite_op_code = :add
+          klass = "del"
+          first = i
+          second = i + 1
+        else
+          opposite_op_code = :delete
+          klass = "ins"
+          first = i + 1
+          second = i
+        end
+
+        if i + 1 < @block_by_block_diff.length && @block_by_block_diff[i + 1][1] == opposite_op_code
+          diff = ONPDiff.new(tokenize_html(@block_by_block_diff[first][0]), tokenize_html(@block_by_block_diff[second][0])).diff
+          inline << generate_inline_html(diff)
+          i += 1
+        else
+          inline << add_class_or_wrap_in_tags(@block_by_block_diff[i][0], klass)
+        end
+      end
+      i += 1
+    end
+
+    "<div class=\"inline-diff\">#{inline.join}</div>"
+  end
+
+  def side_by_side_html
+    i = 0
+    left, right = [], []
+    while i < @block_by_block_diff.length
+      op_code = @block_by_block_diff[i][1]
+      if op_code == :common
+        left << @block_by_block_diff[i][0]
+        right << @block_by_block_diff[i][0]
+      else
+        if op_code == :delete
+          opposite_op_code = :add
+          side = left
+          klass = "del"
+          first = i
+          second = i + 1
+        else
+          opposite_op_code = :delete
+          side = right
+          klass = "ins"
+          first = i + 1
+          second = i
+        end
+
+        if i + 1 < @block_by_block_diff.length && @block_by_block_diff[i + 1][1] == opposite_op_code
+          diff = ONPDiff.new(tokenize_html(@block_by_block_diff[first][0]), tokenize_html(@block_by_block_diff[second][0])).diff
+          deleted, inserted = generate_side_by_side_html(diff)
+          left << deleted
+          right << inserted
+          i += 1
+        else
+          side << add_class_or_wrap_in_tags(@block_by_block_diff[i][0], klass)
+        end
+      end
+      i += 1
+    end
+
+    "<div class=\"span8\">#{left.join}</div><div class=\"span8 offset1\">#{right.join}</div>"
+  end
+
+  def side_by_side_text
+    i = 0
+    table = ["<table class=\"markdown\">"]
+    while i < @line_by_line_diff.length
+      table << "<tr>"
+      op_code = @line_by_line_diff[i][1]
+      if op_code == :common
+        table << "<td>#{CGI::escapeHTML(@line_by_line_diff[i][0])}</td>"
+        table << "<td>#{CGI::escapeHTML(@line_by_line_diff[i][0])}</td>"
+      else
+        if op_code == :delete
+          opposite_op_code = :add
+          first = i
+          second = i + 1
+        else
+          opposite_op_code = :delete
+          first = i + 1
+          second = i
+        end
+
+        if i + 1 < @line_by_line_diff.length && @line_by_line_diff[i + 1][1] == opposite_op_code
+          before_tokens, after_tokens = tokenize_text(@line_by_line_diff[first][0]), tokenize_text(@line_by_line_diff[second][0])
+          if (before_tokens.length - after_tokens.length).abs > MAX_DIFFERENCE
+            before_tokens, after_tokens = tokenize_line(@line_by_line_diff[first][0]), tokenize_line(@line_by_line_diff[second][0])
+          end
+          diff = ONPDiff.new(before_tokens, after_tokens).short_diff
+          deleted, inserted = generate_side_by_side_text(diff)
+          table << "<td class=\"diff-del\">#{deleted.join}</td>"
+          table << "<td class=\"diff-ins\">#{inserted.join}</td>"
+          i += 1
+        else
+          if op_code == :delete
+            table << "<td class=\"diff-del\">#{CGI::escapeHTML(@line_by_line_diff[i][0])}</td>"
+            table << "<td></td>"
+          else
+            table << "<td></td>"
+            table << "<td class=\"diff-ins\">#{CGI::escapeHTML(@line_by_line_diff[i][0])}</td>"
+          end
+        end
+      end
+      table << "</tr>"
+      i += 1
+    end
+    table << "</table>"
+
+    table.join
+  end
+
+  private
+
+  def tokenize_line(text)
+    text.scan(/[^\r\n]+[\r\n]*/)
+  end
+
+  def tokenize_text(text)
+    t, tokens = [], []
+    i = 0
+    while i < text.length
+      if text[i] =~ /\w/
+        t << text[i]
+      elsif text[i] =~ /[ \t]/ && t.join =~ /^\w+$/
+        begin
+          t << text[i]
+          i += 1
+        end while i < text.length && text[i] =~ /[ \t]/
+        i -= 1
+        tokens << t.join
+        t = []
+      else
+        tokens << t.join if t.length > 0
+        tokens << text[i]
+        t = []
+      end
+      i += 1
+    end
+    tokens << t.join if t.length > 0
+    tokens
+  end
+
+  def tokenize_html_blocks(html)
+    Nokogiri::HTML.fragment(html).search("./*").map(&:to_html)
+  end
+
+  def tokenize_html(html)
+    HtmlTokenizer.tokenize(html)
+  end
+
+  def add_class_or_wrap_in_tags(html_or_text, klass)
+    index_of_next_chevron = html_or_text.index(">")
+    if html_or_text.length > 0 && html_or_text[0] == '<' && index_of_next_chevron
+      index_of_class = html_or_text.index("class=")
+      if index_of_class.nil? || index_of_class > index_of_next_chevron
+        # we do not have a class for the current tag
+        # add it right before the ">"
+        html_or_text.insert(index_of_next_chevron, " class=\"diff-#{klass}\"")
+      else
+        # we have a class, insert it at the beginning
+        html_or_text.insert(index_of_class + "class=".length + 1, "diff-#{klass} ")
+      end
+    else
+      "<#{klass}>#{html_or_text}</#{klass}>"
+    end
+  end
+
+  def generate_inline_html(diff)
+    inline = []
+    diff.each do |d|
+      case d[1]
+      when :common then inline << d[0]
+      when :delete then inline << add_class_or_wrap_in_tags(d[0], "del")
+      when :add then inline << add_class_or_wrap_in_tags(d[0], "ins")
+      end
+    end
+    inline
+  end
+
+  def generate_side_by_side_html(diff)
+    deleted, inserted = [], []
+    diff.each do |d|
+      case d[1]
+      when :common
+        deleted << d[0]
+        inserted << d[0]
+      when :delete then deleted << add_class_or_wrap_in_tags(d[0], "del")
+      when :add then inserted << add_class_or_wrap_in_tags(d[0], "ins")
+      end
+    end
+    [deleted, inserted]
+  end
+
+  def generate_side_by_side_text(diff)
+    deleted, inserted = [], []
+    diff.each do |d|
+      case d[1]
+      when :common
+        deleted << d[0]
+        inserted << d[0]
+      when :delete then deleted << "<del>#{CGI::escapeHTML(d[0])}</del>"
+      when :add then inserted << "<ins>#{CGI::escapeHTML(d[0])}</ins>"
+      end
+    end
+    [deleted, inserted]
+  end
+
+  class HtmlTokenizer < Nokogiri::XML::SAX::Document
+
+    attr_accessor :tokens
+
+    def initialize
+      @tokens = []
+    end
+
+    def self.tokenize(html)
+      me = new
+      parser = Nokogiri::HTML::SAX::Parser.new(me)
+      parser.parse("<html><body>#{html}</body></html>")
+      me.tokens
+    end
+
+    USELESS_TAGS = %w{html body}
+    def start_element(name, attributes = [])
+      return if USELESS_TAGS.include?(name)
+      attrs = attributes.map { |a| " #{a[0]}=\"#{a[1]}\"" }.join
+      @tokens << "<#{name}#{attrs}>"
+    end
+
+    AUTOCLOSING_TAGS = %w{area base br col embed hr img input meta}
+    def end_element(name)
+      return if USELESS_TAGS.include?(name) || AUTOCLOSING_TAGS.include?(name)
+      @tokens << "</#{name}>"
+    end
+
+    def characters(string)
+      @tokens.concat string.scan(/(\W|\w+[ \t]*)/).flatten
+    end
+
+  end
+
+end

lib/guardian.rb

@@ -376,6 +376,10 @@ class Guardian
     post.present? && (is_staff? || (!post.deleted_at.present? && can_see_topic?(post.topic)))
   end
 
+  def can_see_post_revision?(post_revision)
+    post_revision.present? && (is_staff? || can_see_post?(post_revision.post))
+  end
+
   def can_see_category?(category)
     not(category.read_restricted) || secure_category_ids.include?(category.id)
   end

lib/onpdiff.rb

@@ -0,0 +1,153 @@
+# Use "An O(NP) Sequence Comparison Algorithm" as described by Sun Wu, Udi Manber and Gene Myers
+# in http://www.itu.dk/stud/speciale/bepjea/xwebtex/litt/an-onp-sequence-comparison-algorithm.pdf
+class ONPDiff
+
+  def initialize(a, b)
+    @a, @b = a, b
+    @m, @n = a.length, b.length
+    @backtrack = []
+    if @reverse = @m > @n
+      @a, @b = @b, @a
+      @m, @n = @n, @m
+    end
+    @offset = @m + 1
+    @delta = @n - @m
+  end
+
+  def diff
+    @diff ||= build_diff_script(compose)
+  end
+
+  def short_diff
+    @short_diff ||= build_short_diff_script(compose)
+  end
+
+  private
+
+  def compose
+    return @shortest_path if @shortest_path
+
+    size = @m + @n + 3
+    fp = Array.new(size) { |i| -1 }
+    @path = Array.new(size) { |i| -1 }
+    p = -1
+
+    begin
+      p += 1
+
+      k = -p
+      while k <= @delta - 1
+        fp[k + @offset] = snake(k, fp[k - 1 + @offset] + 1, fp[k + 1 + @offset])
+        k += 1
+      end
+
+      k = @delta + p
+      while k >= @delta + 1
+        fp[k + @offset] = snake(k, fp[k - 1 + @offset] + 1, fp[k + 1 + @offset])
+        k -= 1
+      end
+
+      fp[@delta + @offset] = snake(@delta, fp[@delta - 1 + @offset] + 1, fp[@delta + 1 + @offset])
+
+    end until fp[@delta + @offset] == @n
+
+    r = @path[@delta + @offset]
+
+    @shortest_path = []
+    while r != -1
+      @shortest_path << [@backtrack[r][0], @backtrack[r][1]]
+      r = @backtrack[r][2]
+    end
+
+    @shortest_path
+  end
+
+  def snake(k, p, pp)
+    r = p > pp ? @path[k - 1 + @offset] : @path[k + 1 + @offset]
+    y = [p, pp].max
+    x = y - k
+
+    while x < @m && y < @n && @a[x] == @b[y]
+      x += 1
+      y += 1
+    end
+
+    @path[k + @offset] = @backtrack.length
+    @backtrack << [x, y, r]
+
+    y
+  end
+
+  def build_diff_script(shortest_path)
+    ses = []
+    x, y = 1, 1
+    px, py = 0, 0
+    i = shortest_path.length - 1
+    while i >= 0
+      while px < shortest_path[i][0] || py < shortest_path[i][1]
+        if shortest_path[i][1] - shortest_path[i][0] > py - px
+          t = @reverse ? :delete : :add
+          ses << [@b[py], t]
+          y += 1
+          py += 1
+        elsif shortest_path[i][1] - shortest_path[i][0] < py - px
+          t = @reverse ? :add : :delete
+          ses << [@a[px], t]
+          x += 1
+          px += 1
+        else
+          ses << [@a[px], :common]
+          x += 1
+          y += 1
+          px += 1
+          py += 1
+        end
+      end
+      i -= 1
+    end
+    ses
+  end
+
+  def build_short_diff_script(shortest_path)
+    ses = []
+    x, y = 1, 1
+    px, py = 0, 0
+    i = shortest_path.length - 1
+    while i >= 0
+      while px < shortest_path[i][0] || py < shortest_path[i][1]
+        if shortest_path[i][1] - shortest_path[i][0] > py - px
+          t = @reverse ? :delete : :add
+          if ses.length > 0 && ses[-1][1] == t
+            ses[-1][0] << @b[py]
+          else
+            ses << [@b[py], t]
+          end
+          y += 1
+          py += 1
+        elsif shortest_path[i][1] - shortest_path[i][0] < py - px
+          t = @reverse ? :add : :delete
+          if ses.length > 0 && ses[-1][1] == t
+            ses[-1][0] << @a[px]
+          else
+            ses << [@a[px], t]
+          end
+          x += 1
+          px += 1
+        else
+          if ses.length > 0 && ses[-1][1] == :common
+            ses[-1][0] << @a[px]
+          else
+            ses << [@a[px], :common]
+          end
+          x += 1
+          y += 1
+          px += 1
+          py += 1
+        end
+      end
+      i -= 1
+    end
+    ses
+  end
+
+end

lib/post_destroyer.rb

@@ -124,7 +124,7 @@ class PostDestroyer
     Post.transaction do
       @post.update_column(:user_deleted, false)
       @post.skip_unique_check = true
-      @post.revise(@user, @post.versions.last.modifications["raw"][0], force_new_version: true)
+      @post.revise(@user, @post.revisions.last.modifications["raw"][0], force_new_version: true)
       @post.update_flagged_posts_count
     end
   end

lib/post_revisor.rb

@@ -1,4 +1,5 @@
 require 'edit_rate_limiter'
+
 class PostRevisor
 
   attr_reader :category_changed
@@ -12,12 +13,12 @@ class PostRevisor
     return false if not should_revise?
 
     @post.acting_user = @user
-    @post.updated_by = @user
     revise_post
     update_category_description
     post_process_post
     update_topic_word_counts
     @post.advance_draft_sequence
+
     true
   end
 
@@ -31,7 +32,7 @@ class PostRevisor
     if should_create_new_version?
       revise_and_create_new_version
     else
-      revise_without_creating_a_new_version
+      update_post
     end
   end
 
@@ -47,7 +48,7 @@ class PostRevisor
 
   def revise_and_create_new_version
     Post.transaction do
-      @post.cached_version = @post.version + 1
+      @post.version += 1
       @post.last_version_at = get_revised_at
       update_post
       EditRateLimiter.new(@post.user).performed! unless @opts[:bypass_rate_limiter] == true
@@ -55,12 +56,6 @@ class PostRevisor
     end
   end
 
-  def revise_without_creating_a_new_version
-    @post.skip_version do
-      update_post
-    end
-  end
-
   def bump_topic
     unless Post.where('post_number > ? and topic_id = ?', @post.post_number, @post.topic_id).exists?
       @post.topic.update_column(:bumped_at, Time.now)
@@ -76,7 +71,6 @@ class PostRevisor
   def update_post
     @post.raw = @new_raw
     @post.word_count = @new_raw.scan(/\w+/).size
-    @post.updated_by = @user
     @post.last_editor_id = @user.id
     @post.edit_reason = @opts[:edit_reason] if @opts[:edit_reason]
 

spec/components/cooked_post_processor_spec.rb

@@ -296,7 +296,7 @@ describe CookedPostProcessor do
       before { SiteSetting.stubs(:download_remote_images_to_local).returns(true) }
 
       it "runs only when a user updated the post" do
-        post.updated_by = Discourse.system_user
+        post.last_editor_id = Discourse.system_user.id
         Jobs.expects(:cancel_scheduled_job).never
         cpp.pull_hotlinked_images
       end

spec/components/diff_engine_spec.rb

@@ -1,58 +0,0 @@
-require 'spec_helper'
-require 'diff_engine'
-
-describe DiffEngine do
-
-  let(:html_before) do
-    <<-HTML.strip_heredoc
-    <context>
-    <original>text</original>
-    </context>
-    HTML
-  end
-
-  let(:markdown_special_characters) do
-    "=\`*_{}[]()#+-.!"
-  end
-
-  it "escapes input html to markup with diff html" do
-    diff = DiffEngine.html_diff("<html>", "")
-
-    diff.should include("&lt;html&gt;")
-  end
-
-  it "generates an html diff with ins and dels for changed" do
-    html_after = html_before
-      .gsub(/original/, "changed")
-
-    diff = DiffEngine.html_diff(html_before, html_after)
-
-    diff.should match(/del.*?original.*?del/)
-    diff.should match(/ins.*?changed.*?ins/)
-  end
-
-  it "generates an html diff with only ins for inserted" do
-    html_after = "#{html_before}\nnew"
-
-    diff = DiffEngine.html_diff(html_before, html_after)
-
-    diff.should include("ins")
-    diff.should_not include("del")
-  end
-
-  it "generates an html diff with only unchanged for unchanged" do
-    html_after = html_before
-
-    diff = DiffEngine.html_diff(html_before, html_after)
-
-    diff.should include("unchanged")
-    diff.should_not include("del", "ins")
-  end
-
-  it "handles markdown special characters" do
-    diff = DiffEngine.markdown_diff(markdown_special_characters, "")
-
-    diff.should include(markdown_special_characters)
-  end
-
-end

spec/components/post_revisor_spec.rb

@@ -19,8 +19,8 @@ describe PostRevisor do
         subject.revise!(post.user, post.raw).should be_false
       end
 
-      it "doesn't change cached_version" do
+      it "doesn't change version" do
-        lambda { subject.revise!(post.user, post.raw); post.reload }.should_not change(post, :cached_version)
+        lambda { subject.revise!(post.user, post.raw); post.reload }.should_not change(post, :version)
       end
 
     end
@@ -32,12 +32,12 @@ describe PostRevisor do
         post.reload
       end
 
-      it 'does not update cached_version' do
+      it 'does not update version' do
-        post.cached_version.should == 1
+        post.version.should == 1
       end
 
-      it 'does not create a new version' do
+      it 'does not create a new revision' do
-        post.all_versions.size.should == 1
+        post.revisions.size.should == 0
       end
 
       it "doesn't change the last_version_at" do
@@ -64,12 +64,12 @@ describe PostRevisor do
         subject.category_changed.should be_blank
       end
 
-      it 'updates the cached_version' do
+      it 'updates the version' do
-        post.cached_version.should == 2
+        post.version.should == 2
       end
 
       it 'creates a new version' do
-        post.all_versions.size.should == 2
+        post.revisions.size.should == 1
       end
 
       it "updates the last_version_at" do
@@ -84,7 +84,7 @@ describe PostRevisor do
         end
 
         it "doesn't create a new version if you do another" do
-          post.cached_version.should == 2
+          post.version.should == 2
         end
 
         it "doesn't change last_version_at" do
@@ -105,7 +105,7 @@ describe PostRevisor do
           end
 
           it "does create a new version after the edit window" do
-            post.cached_version.should == 3
+            post.version.should == 3
           end
 
           it "does create a new version after the edit window" do
@@ -199,7 +199,7 @@ describe PostRevisor do
       end
 
       it "marks the admin as the last updater" do
-        post.updated_by.should == changed_by
+        post.last_editor_id.should == changed_by.id
       end
 
     end
@@ -236,20 +236,16 @@ describe PostRevisor do
         post.invalidate_oneboxes.should == true
       end
 
-      it 'increased the cached_version' do
+      it 'increased the version' do
-        post.cached_version.should == 2
+        post.version.should == 2
       end
 
-      it 'has the new version in all_versions' do
+      it 'has the new revision' do
-        post.all_versions.size.should == 2
+        post.revisions.size.should == 1
       end
 
-      it 'has versions' do
+      it "saved the user who made the change in the revisions" do
-        post.versions.should be_present
+        post.revisions.first.user_id.should == changed_by.id
-      end
-
-      it "saved the user who made the change in the version" do
-        post.versions.first.user.should be_present
       end
 
       it "updates the word count" do
@@ -266,11 +262,11 @@ describe PostRevisor do
         end
 
         it 'is a ninja edit, because the second poster posted again quickly' do
-          post.cached_version.should == 2
+          post.version.should == 2
         end
 
         it 'is a ninja edit, because the second poster posted again quickly' do
-          post.all_versions.size.should == 2
+          post.revisions.size.should == 1
         end
       end
     end

spec/controllers/posts_controller_spec.rb

@@ -70,34 +70,6 @@ describe PostsController do
     end
   end
 
-
-  describe 'versions' do
-
-    shared_examples 'posts_controller versions examples' do
-      it "raises an error if the user doesn't have permission to see the post" do
-        Guardian.any_instance.expects(:can_see?).with(post).returns(false)
-        xhr :get, :versions, post_id: post.id
-        response.should be_forbidden
-      end
-
-      it 'renders JSON' do
-        xhr :get, :versions, post_id: post.id
-        ::JSON.parse(response.body).should be_present
-      end
-    end
-
-    context 'when not logged in' do
-      let(:post) { Fabricate(:post) }
-      include_examples 'posts_controller versions examples'
-    end
-
-    context 'when logged in' do
-      let(:post) { Fabricate(:post, user: log_in) }
-      include_examples 'posts_controller versions examples'
-    end
-
-  end
-
   describe 'delete a post' do
     it 'raises an exception when not logged in' do
       lambda { xhr :delete, :destroy, id: 123 }.should raise_error(Discourse::NotLoggedIn)

spec/models/post_spec.rb

@@ -28,6 +28,9 @@ describe Post do
 
   it { should have_many :post_details }
 
+  it { should have_many :post_revisions }
+  it { should have_many :revisions}
+
   it { should rate_limit }
 
   let(:topic) { Fabricate(:topic) }
@@ -35,8 +38,6 @@ describe Post do
     { user: topic.user, topic: topic }
   end
 
-  it_behaves_like "a versioned model"
-
   describe 'scopes' do
 
     describe '#by_newest' do
@@ -57,7 +58,7 @@ describe Post do
 
   end
 
-  describe "versions and deleting/recovery" do
+  describe "revisions and deleting/recovery" do
 
     context 'a post without links' do
       let(:post) { Fabricate(:post, post_args) }
@@ -67,8 +68,8 @@ describe Post do
         post.reload
       end
 
-      it "doesn't create a new version when deleted" do
+      it "doesn't create a new revision when deleted" do
-        post.versions.count.should == 0
+        post.revisions.count.should == 0
       end
 
       describe "recovery" do
@@ -77,8 +78,8 @@ describe Post do
           post.reload
         end
 
-        it "doesn't create a new version when recovered" do
+        it "doesn't create a new revision when recovered" do
-          post.versions.count.should == 0
+          post.revisions.count.should == 0
         end
       end
     end
@@ -481,16 +482,16 @@ describe Post do
     let(:post) { Fabricate(:post, post_args) }
     let(:first_version_at) { post.last_version_at }
 
-    it 'has one version in all_versions' do
+    it 'has no revision' do
-      post.all_versions.size.should == 1
+      post.revisions.size.should == 0
       first_version_at.should be_present
       post.revise(post.user, post.raw).should be_false
     end
 
-
     describe 'with the same body' do
-      it "doesn't change cached_version" do
+
-        lambda { post.revise(post.user, post.raw); post.reload }.should_not change(post, :cached_version)
+      it "doesn't change version" do
+        lambda { post.revise(post.user, post.raw); post.reload }.should_not change(post, :version)
       end
 
     end
@@ -503,8 +504,8 @@ describe Post do
       end
 
       it 'causes no update' do
-        post.cached_version.should == 1
+        post.version.should == 1
-        post.all_versions.size.should == 1
+        post.revisions.size.should == 0
         post.last_version_at.should == first_version_at
       end
 
@@ -520,9 +521,9 @@ describe Post do
         post.reload
       end
 
-      it 'updates the cached_version' do
+      it 'updates the version' do
-        post.cached_version.should == 2
+        post.version.should == 2
-        post.all_versions.size.should == 2
+        post.revisions.size.should == 1
         post.last_version_at.to_i.should == revised_at.to_i
       end
 
@@ -534,7 +535,7 @@ describe Post do
         end
 
         it "doesn't create a new version if you do another" do
-          post.cached_version.should == 2
+          post.version.should == 2
         end
 
         it "doesn't change last_version_at" do
@@ -551,7 +552,7 @@ describe Post do
           end
 
           it "does create a new version after the edit window" do
-            post.cached_version.should == 3
+            post.version.should == 3
           end
 
           it "does create a new version after the edit window" do
@@ -582,10 +583,9 @@ describe Post do
         result.should be_true
         post.raw.should == 'updated body'
         post.invalidate_oneboxes.should == true
-        post.cached_version.should == 2
+        post.version.should == 2
-        post.all_versions.size.should == 2
+        post.revisions.size.should == 1
-        post.versions.should be_present
+        post.revisions.first.user.should be_present
-        post.versions.first.user.should be_present
       end
 
       context 'second poster posts again quickly' do
@@ -596,8 +596,8 @@ describe Post do
         end
 
         it 'is a ninja edit, because the second poster posted again quickly' do
-          post.cached_version.should == 2
+          post.version.should == 2
-          post.all_versions.size.should == 2
+          post.revisions.size.should == 1
         end
 
       end
@@ -615,7 +615,7 @@ describe Post do
       post.post_number.should be_present
       post.excerpt.should be_present
       post.post_type.should == Post.types[:regular]
-      post.versions.should be_blank
+      post.revisions.should be_blank
       post.cooked.should be_present
       post.external_id.should be_present
       post.quote_count.should == 0

spec/models/topic_spec.rb

@@ -21,11 +21,11 @@ describe Topic do
   it { should have_many :topic_allowed_users }
   it { should have_many :allowed_users }
   it { should have_many :invites }
+  it { should have_many :topic_revisions }
+  it { should have_many :revisions }
 
   it { should rate_limit }
 
-  it_behaves_like "a versioned model"
-
   context 'slug' do
 
     let(:title) { "hello world topic" }
@@ -734,22 +734,24 @@ describe Topic do
     end
   end
 
-  describe 'versions' do
+  describe 'revisions' do
     let(:topic) { Fabricate(:topic) }
 
-    it "has version 1 by default" do
+    it "has no revisions by default" do
-      topic.version.should == 1
+      topic.revisions.size.should == 1
     end
 
     context 'changing title' do
+
       before do
         topic.title = "new title for the topic"
         topic.save
       end
 
-      it "creates a new version" do
+      it "creates a new revision" do
-        topic.version.should == 2
+        topic.revisions.size.should == 2
       end
+
     end
 
     context 'changing category' do
@@ -759,8 +761,8 @@ describe Topic do
         topic.change_category(category.name)
       end
 
-      it "creates a new version" do
+      it "creates a new revision" do
-        topic.version.should == 2
+        topic.revisions.size.should == 2
       end
 
       context "removing a category" do
@@ -768,8 +770,8 @@ describe Topic do
           topic.change_category(nil)
         end
 
-        it "creates a new version" do
+        it "creates a new revision" do
-          topic.version.should == 3
+          topic.revisions.size.should == 3
         end
       end
 
@@ -781,8 +783,8 @@ describe Topic do
         topic.save
       end
 
-      it "doesn't craete a new version" do
+      it "doesn't create a new version" do
-        topic.version.should == 1
+        topic.revisions.size.should == 1
       end
     end