diff --git a/app/assets/javascripts/discourse/controllers/history_controller.js b/app/assets/javascripts/discourse/controllers/history_controller.js
index a62bd62b9..62481df8c 100644
--- a/app/assets/javascripts/discourse/controllers/history_controller.js
+++ b/app/assets/javascripts/discourse/controllers/history_controller.js
@@ -38,7 +38,45 @@ Discourse.HistoryController = Discourse.ObjectController.extend(Discourse.ModalF
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"),
+ category_diff: function() {
+ var viewMode = this.get("viewMode");
+ var changes = this.get("category_changes");
+
+ var prevCategory = Discourse.Category.findById(changes.previous_category_id);
+ var curCategory = Discourse.Category.findById(changes.current_category_id);
+
+ var raw = "";
+
+ prevCategory = Discourse.HTML.categoryLink(prevCategory);
+ curCategory = Discourse.HTML.categoryLink(curCategory);
+
+ if(viewMode === "side_by_side_markdown" || viewMode === "side_by_side") {
+ raw = "
" + prevCategory + "
" + curCategory + "
";
+ } else {
+ var diff;
+ if(curCategory === prevCategory){
+ diff = curCategory;
+ } else {
+ diff = "" + prevCategory + " " + "" + curCategory + "";
+ }
+ raw = "" + diff + "
";
+ }
+
+ return raw;
+
+ }.property("inline", "side_by_side", "side_by_side_markdown", "viewMode"),
+
+ title_diff: function() {
+ var viewMode = this.get("viewMode");
+ if(viewMode === "side_by_side_markdown") {
+ viewMode = "side_by_side";
+ }
+ return this.get("title_changes." + viewMode);
+ }.property("inline", "side_by_side", "side_by_side_markdown", "viewMode"),
+
+ body_diff: function() {
+ return this.get("body_changes." + this.get("viewMode"));
+ }.property("inline", "side_by_side", "side_by_side_markdown", "viewMode"),
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 a4bb40bce..e092e9a8a 100644
--- a/app/assets/javascripts/discourse/templates/modal/history.js.handlebars
+++ b/app/assets/javascripts/discourse/templates/modal/history.js.handlebars
@@ -22,7 +22,15 @@
{{i18n post.revisions.details.edited_by}} {{avatar this imageSize="small"}} {{username}} {{unboundDate path="created_at" leaveAgo="true"}} {{#if edit_reason}} — {{edit_reason}}{{/if}}
- {{{diff}}}
+ {{#if title_changes}}
+
{{{title_diff}}}
+ {{/if}}
+ {{#if category_changes}}
+
+ {{{category_diff}}}
+
+ {{/if}}
+ {{{body_diff}}}
{{/if}}
diff --git a/app/models/post_revision.rb b/app/models/post_revision.rb
index fd46c90e7..d3b74a374 100644
--- a/app/models/post_revision.rb
+++ b/app/models/post_revision.rb
@@ -1,8 +1,96 @@
+require_dependency "discourse_diff"
+
class PostRevision < ActiveRecord::Base
belongs_to :post
belongs_to :user
serialize :modifications, Hash
+
+ def body_changes
+ changes_for("cooked", "raw")
+ end
+
+ def category_changes
+ {
+ previous_category_id: previous("category_id"),
+ current_category_id: current("category_id"),
+ }
+ 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 = "#{CGI::escapeHTML(prev)}
"
+ cur = "#{CGI::escapeHTML(cur)}
"
+ end
+
+ 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)
+ lookup_with_fallback(field, 0)
+ end
+
+ def current(field)
+ lookup_with_fallback(field, 1)
+ end
+
+ def previous_revisions
+ @previous_revs ||=
+ PostRevision.where("post_id = ? AND number < ?",
+ post_id, number
+ )
+ .order("number desc")
+ .to_a
+ end
+
+ def has_topic_data?
+ post && post.post_number == 1
+ end
+
+ def lookup_with_fallback(field, index)
+
+ unless val = lookup(field, index)
+ previous_revisions.each do |v|
+ break if val = v.lookup(field, 1)
+ end
+ end
+
+ unless val
+ if ["cooked","raw"].include?(field)
+ val = post.send(field)
+ else
+ val = post.topic.send(field)
+ end
+ end
+
+ val
+ end
+
+ def lookup(field, index)
+ if mod = modifications[field]
+ mod[index]
+ end
+ end
+
end
# == Schema Information
diff --git a/app/models/topic.rb b/app/models/topic.rb
index d3bfe1136..34dcd725c 100644
--- a/app/models/topic.rb
+++ b/app/models/topic.rb
@@ -92,7 +92,6 @@ 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)
@@ -189,13 +188,20 @@ class Topic < ActiveRecord::Base
end
+ # TODO move into PostRevisor or TopicRevisor
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)
- )
+ if first_post_id = posts.where(post_number: 1).pluck(:id).first
+
+ number = PostRevision.where(post_id: first_post_id).count + 2
+ PostRevision.create!(
+ user_id: acting_user.id,
+ post_id: first_post_id,
+ number: number,
+ modifications: changes.extract!(:category_id, :title)
+ )
+
+ Post.update_all({version: number}, id: first_post_id)
+ end
end
def should_create_new_version?
diff --git a/app/models/topic_revision.rb b/app/models/topic_revision.rb
deleted file mode 100644
index 5190881e4..000000000
--- a/app/models/topic_revision.rb
+++ /dev/null
@@ -1,24 +0,0 @@
-class TopicRevision < ActiveRecord::Base
- belongs_to :topic
- belongs_to :user
-
- serialize :modifications, Hash
-end
-
-# == Schema Information
-#
-# Table name: topic_revisions
-#
-# id :integer not null, primary key
-# user_id :integer
-# topic_id :integer
-# modifications :text
-# number :integer
-# created_at :datetime
-# updated_at :datetime
-#
-# Indexes
-#
-# index_topic_revisions_on_topic_id (topic_id)
-# index_topic_revisions_on_topic_id_and_number (topic_id,number)
-#
diff --git a/app/serializers/post_revision_serializer.rb b/app/serializers/post_revision_serializer.rb
index fc3ccef97..8d167ad81 100644
--- a/app/serializers/post_revision_serializer.rb
+++ b/app/serializers/post_revision_serializer.rb
@@ -1,5 +1,3 @@
-require_dependency "discourse_diff"
-
class PostRevisionSerializer < ApplicationSerializer
attributes :post_id,
:version,
@@ -9,9 +7,17 @@ class PostRevisionSerializer < ApplicationSerializer
:avatar_template,
:created_at,
:edit_reason,
- :inline,
- :side_by_side,
- :side_by_side_markdown
+ :body_changes,
+ :title_changes,
+ :category_changes
+
+ def include_title_changes?
+ object.has_topic_data?
+ end
+
+ def include_category_changes?
+ object.has_topic_data?
+ end
def version
object.number
@@ -22,50 +28,24 @@ class PostRevisionSerializer < ApplicationSerializer
end
def username
- object.user.username_lower
+ user.username_lower
end
def display_username
- object.user.username
+ user.username
end
def avatar_template
- object.user.avatar_template
+ user.avatar_template
end
def edit_reason
- return unless object.modifications["edit_reason"].present?
- object.modifications["edit_reason"][1]
+ object.lookup("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_markdown
- 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]
+ def user
+ # if stuff goes pear shape attribute to system
+ object.user || Discourse.system_user
end
end
diff --git a/db/migrate/20140306223522_move_topic_revisions_to_post_revisions.rb b/db/migrate/20140306223522_move_topic_revisions_to_post_revisions.rb
new file mode 100644
index 000000000..2f5cd70b0
--- /dev/null
+++ b/db/migrate/20140306223522_move_topic_revisions_to_post_revisions.rb
@@ -0,0 +1,43 @@
+class MoveTopicRevisionsToPostRevisions < ActiveRecord::Migration
+ def up
+ execute < ["bar", "bar1"]})
+ p.previous("foo").should == "bar"
+ p.current("foo").should == "bar1"
+ end
+
+ it "can fallback to previous revisions if needed" do
+ create_rev("foo" => ["A", "B"])
+ r2 = create_rev("bar" => ["C", "D"])
+
+ r2.current("foo").should == "B"
+ r2.previous("foo").should == "B"
+ end
+
+ it "can fallback to post if needed" do
+ post = Fabricate(:post)
+ r = create_rev({"foo" => ["A", "B"]}, post.id)
+
+ r.current("raw").should == post.raw
+ r.previous("raw").should == post.raw
+ r.current("cooked").should == post.cooked
+ r.previous("cooked").should == post.cooked
+ end
+
+ it "can fallback to topic if needed" do
+ post = Fabricate(:post)
+ r = create_rev({"foo" => ["A", "B"]}, post.id)
+
+ r.current("title").should == post.topic.title
+ r.previous("title").should == post.topic.title
+ end
+
+ it "can find title changes" do
+ r = create_rev({"title" => ["hello", "frog"]})
+ r.title_changes[:inline].should =~ /frog.*hello/
+ r.title_changes[:side_by_side].should =~ /hello.*frog/
+ end
+
+ it "can find category changes" do
+ cat1 = Fabricate(:category, name: "cat1")
+ cat2 = Fabricate(:category, name: "cat2")
+
+ r = create_rev({"category_id" => [cat1.id, cat2.id]})
+
+ changes = r.category_changes
+ changes[:previous_category_id].should == cat1.id
+ changes[:current_category_id].should == cat2.id
+
+ end
end
diff --git a/spec/models/topic_revision_spec.rb b/spec/models/topic_revision_spec.rb
deleted file mode 100644
index f1717ea9e..000000000
--- a/spec/models/topic_revision_spec.rb
+++ /dev/null
@@ -1,9 +0,0 @@
-require 'spec_helper'
-require_dependency 'topic_revision'
-
-describe TopicRevision do
-
- it { should belong_to :user }
- it { should belong_to :topic }
-
-end
diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb
index 1595b993b..6d5bbf8f0 100644
--- a/spec/models/topic_spec.rb
+++ b/spec/models/topic_spec.rb
@@ -7,23 +7,6 @@ describe Topic do
it { should validate_presence_of :title }
- it { should belong_to :category }
- it { should belong_to :user }
- it { should belong_to :last_poster }
- it { should belong_to :featured_user1 }
- it { should belong_to :featured_user2 }
- it { should belong_to :featured_user3 }
- it { should belong_to :featured_user4 }
-
- it { should have_many :posts }
- it { should have_many :topic_users }
- it { should have_many :topic_links }
- 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 }
context 'slug' do
@@ -735,10 +718,11 @@ describe Topic do
end
describe 'revisions' do
- let(:topic) { Fabricate(:topic) }
+ let(:post) { Fabricate(:post) }
+ let(:topic) { post.topic }
it "has no revisions by default" do
- topic.revisions.size.should == 1
+ post.revisions.size.should == 0
end
context 'changing title' do
@@ -749,7 +733,7 @@ describe Topic do
end
it "creates a new revision" do
- topic.revisions.size.should == 2
+ post.revisions.size.should == 1
end
end
@@ -762,7 +746,7 @@ describe Topic do
end
it "creates a new revision" do
- topic.revisions.size.should == 2
+ post.revisions.size.should == 1
end
context "removing a category" do
@@ -771,7 +755,12 @@ describe Topic do
end
it "creates a new revision" do
- topic.revisions.size.should == 3
+ post.revisions.size.should == 2
+ last_rev = post.revisions.order(:number).last
+ last_rev.previous("category_id").should == category.id
+ last_rev.current("category_id").should == SiteSetting.uncategorized_category_id
+ post.reload
+ post.version.should == 3
end
end
@@ -784,7 +773,7 @@ describe Topic do
end
it "doesn't create a new version" do
- topic.revisions.size.should == 1
+ post.revisions.size.should == 0
end
end
@@ -1210,7 +1199,7 @@ describe Topic do
describe 'secured' do
it 'can remove secure groups' do
category = Fabricate(:category, read_restricted: true)
- topic = Fabricate(:topic, category: category)
+ Fabricate(:topic, category: category)
Topic.secured(Guardian.new(nil)).count.should == 0
Topic.secured(Guardian.new(Fabricate(:admin))).count.should == 2