Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

LOTS of changes to properly handle post/topic revisions 

FIX: history revision can now properly be hidden
FIX: PostRevision serializer is now entirely dynamic to properly handle
hidden revisions
FIX: default history modal to "side by side" view on mobile
FIX: properly hiden which revision has been hidden
UX: inline category/user/wiki/post_type changes with the revision
details
FEATURE: new '/posts/:post_id/revisions/latest' endpoint to retrieve
latest revision
UX: do not show the hide/show revision button on mobile (no room for
them)
UX: remove CSS transitions on the buttons in the history modal
FIX: PostRevisor now handles all the changes that might create new
revisions
FIX: PostRevision.ensure_consistency! was wrong due to off by 1
mistake...
refactored topic's callbacks for better readability
extracted 'PostRevisionGuardian'
This commit is contained in:
Régis Hanol 2014-10-27 22:06:43 +01:00
parent caf31dde1e
commit e7f251c105
46 changed files with 878 additions and 949 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

123
app/assets/javascripts/discourse/controllers/history.js.es6
View File

@ -12,8 +12,8 @@ import ObjectController from 'discourse/controllers/object';
@module Discourse
**/
export default ObjectController.extend(ModalFunctionality, {
loading: false,
viewMode: "side_by_side",
loading: true,
viewMode: Discourse.Mobile.mobileView ? "inline" : "side_by_side",
revisionsTextKey: "post.revisions.controls.comparing_previous_to_current_out_of_total",
refresh: function(postId, postVersion) {
@ -41,87 +41,72 @@ export default ObjectController.extend(ModalFunctionality, {
createdAtDate: function() { return moment(this.get("created_at")).format("LLLL"); }.property("created_at"),
previousVersion: function() { return this.get("version") - 1; }.property("version"),
previousVersion: function() { return this.get("current_version") - 1; }.property("current_version"),
displayGoToFirst: Em.computed.gt("version", 3),
displayGoToPrevious: Em.computed.gt("version", 2),
displayRevisions: Em.computed.gt("revisions_count", 2),
displayGoToNext: function() { return this.get("version") < this.get("revisions_count"); }.property("version", "revisions_count"),
displayGoToLast: function() { return this.get("version") < this.get("revisions_count") - 1; }.property("version", "revisions_count"),
displayGoToFirst: function() { return this.get("current_revision") > this.get("first_revision"); }.property("current_revision", "first_revision"),
displayGoToPrevious: function() { return this.get("previous_revision") && this.get("current_revision") > this.get("previous_revision"); }.property("current_revision", "previous_revision"),
displayRevisions: Em.computed.gt("version_count", 2),
displayGoToNext: function() { return this.get("next_revision") && this.get("current_revision") < this.get("next_revision"); }.property("current_revision", "next_revision"),
displayGoToLast: function() { return this.get("current_revision") < this.get("last_revision"); }.property("current_revision", "last_revision"),
displayShow: function() { return this.get("hidden") && Discourse.User.currentProp('staff'); }.property("hidden"),
displayHide: function() { return !this.get("hidden") && Discourse.User.currentProp('staff'); }.property("hidden"),
displayShow: function() { return !Discourse.Mobile.mobileView && this.get("previous_hidden") && Discourse.User.currentProp('staff'); }.property("previous_hidden"),
displayHide: function() { return !Discourse.Mobile.mobileView && !this.get("previous_hidden") && Discourse.User.currentProp('staff'); }.property("previous_hidden"),
isEitherRevisionHidden: Em.computed.or("previous_hidden", "current_hidden"),
hiddenClasses: function() {
if (this.get("displayingInline")) {
return this.get("isEitherRevisionHidden") ? "hidden-revision-either" : null;
} else {
var result = [];
if (this.get("previous_hidden")) { result.push("hidden-revision-previous"); }
if (this.get("current_hidden")) { result.push("hidden-revision-current"); }
return result.join(" ");
}
}.property("previous_hidden", "current_hidden", "displayingInline"),
displayingInline: Em.computed.equal("viewMode", "inline"),
displayingSideBySide: Em.computed.equal("viewMode", "side_by_side"),
displayingSideBySideMarkdown: Em.computed.equal("viewMode", "side_by_side_markdown"),
category_diff: function() {
var viewMode = this.get("viewMode");
previousCategory: function() {
var changes = this.get("category_changes");
if (changes === null) { return; }
var prevCategory = Discourse.Category.findById(changes.previous_category_id);
var curCategory = Discourse.Category.findById(changes.current_category_id);
var raw = "";
var opts = { allowUncategorized: true };
prevCategory = Discourse.HTML.categoryBadge(prevCategory, opts);
curCategory = Discourse.HTML.categoryBadge(curCategory, opts);
if(viewMode === "side_by_side_markdown" || viewMode === "side_by_side") {
raw = "<div class='span8'>" + prevCategory + "</div> <div class='span8 offset1'>" + curCategory + "</div>";
} else {
var diff = "<del>" + prevCategory + "</del> " + "<ins>" + curCategory + "</ins>";
raw = "<div class='inline-diff'>" + diff + "</div>";
if (changes) {
var category = Discourse.Category.findById(changes["previous"]);
return Discourse.HTML.categoryBadge(category, { allowUncategorized: true });
}
}.property("category_changes"),
return raw;
}.property("viewMode", "category_changes"),
currentCategory: function() {
var changes = this.get("category_changes");
if (changes) {
var category = Discourse.Category.findById(changes["current"]);
return Discourse.HTML.categoryBadge(category, { allowUncategorized: true });
}
}.property("category_changes"),
wiki_diff: function() {
var viewMode = this.get("viewMode");
var changes = this.get("wiki_changes");
if (changes === null) { return; }
if (viewMode === "inline") {
var diff = changes["current_wiki"] ? '<i class="fa fa-pencil-square-o fa-2x"></i>' : '<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>';
return "<div class='inline-diff'>" + diff + "</div>";
} else {
var prev = changes["previous_wiki"] ? '<i class="fa fa-pencil-square-o fa-2x"></i>' : "&nbsp;";
var curr = changes["current_wiki"] ? '<i class="fa fa-pencil-square-o fa-2x"></i>' : '<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>';
return "<div class='span8'>" + prev + "</div><div class='span8 offset1'>" + curr + "</div>";
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("viewMode", "wiki_changes"),
}.property("wiki_changes"),
post_type_diff: function () {
var viewMode = this.get("viewMode");
var changes = this.get("post_type_changes");
if (changes === null) { return; }
var moderator = Discourse.Site.currentProp('post_types.moderator_action');
if (viewMode === "inline") {
var diff = changes["current_post_type"] === moderator ?
'<i class="fa fa-shield fa-2x"></i>' :
'<span class="fa-stack"><i class="fa fa-shield fa-stack-2x"></i><i class="fa fa-ban fa-stack-2x"></i></span>';
return "<div class='inline-diff'>" + diff + "</div>";
} else {
var prev = changes["previous_post_type"] === moderator ? '<i class="fa fa-shield fa-2x"></i>' : "&nbsp;";
var curr = changes["current_post_type"] === moderator ?
'<i class="fa fa-shield fa-2x"></i>' :
'<span class="fa-stack"><i class="fa fa-shield fa-stack-2x"></i><i class="fa fa-ban fa-stack-2x"></i></span>';
return "<div class='span8'>" + prev + "</div><div class='span8 offset1'>" + curr + "</div>";
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>';
}
}.property("viewMode", "post_type_changes"),
}.property("post_type_changes"),
title_diff: function() {
var viewMode = this.get("viewMode");
if(viewMode === "side_by_side_markdown") {
viewMode = "side_by_side";
}
if (viewMode === "side_by_side_markdown") { viewMode = "side_by_side"; }
return this.get("title_changes." + viewMode);
}.property("viewMode", "title_changes"),
@ -130,13 +115,13 @@ export default ObjectController.extend(ModalFunctionality, {
}.property("viewMode", "body_changes"),
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")); },
loadFirstVersion: function() { this.refresh(this.get("post_id"), this.get("first_revision")); },
loadPreviousVersion: function() { this.refresh(this.get("post_id"), this.get("previous_revision")); },
loadNextVersion: function() { this.refresh(this.get("post_id"), this.get("next_revision")); },
loadLastVersion: function() { this.refresh(this.get("post_id"), this.get("last_revision")); },
hideVersion: function() { this.hide(this.get("post_id"), this.get("version")); },
showVersion: function() { this.show(this.get("post_id"), this.get("version")); },
hideVersion: function() { this.hide(this.get("post_id"), this.get("current_revision")); },
showVersion: function() { this.show(this.get("post_id"), this.get("current_revision")); },
displayInline: function() { this.set("viewMode", "inline"); },
displaySideBySide: function() { this.set("viewMode", "side_by_side"); },

4
app/assets/javascripts/discourse/models/_post.js
View File

@ -111,9 +111,7 @@ Discourse.Post = Discourse.Model.extend({
}.property('link_counts.@each.internal'),
// Edits are the version - 1, so version 2 = 1 edit
editCount: function() {
return this.get('version') - 1;
}.property('version'),
editCount: function() { return this.get('version') - 1; }.property('version'),
flagsAvailable: function() {
var post = this;

2
app/assets/javascripts/discourse/routes/topic_route.js
View File

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

75
app/assets/javascripts/discourse/templates/modal/history.hbs
View File

@ -7,13 +7,13 @@
{{#if loading}}
<div id='revision-loading'><i class='fa fa-spinner fa-spin'></i>{{i18n loading}}</div>
{{else}}
{{boundI18n revisionsTextKey previousBinding="previousVersion" currentBinding="version" totalBinding="revisions_count"}}
{{boundI18n revisionsTextKey previousBinding="previousVersion" currentBinding="current_version" totalBinding="version_count"}}
{{/if}}
</div>
<button title="{{i18n post.revisions.controls.next}}" {{bind-attr class=":btn :standard :no-text displayGoToNext::invisible" disabled=loading}} {{action "loadNextVersion"}}><i class="fa fa-forward"></i></button>
<button title="{{i18n post.revisions.controls.last}}" {{bind-attr class=":btn :standard :no-text displayGoToLast::invisible" disabled=loading}} {{action "loadLastVersion"}}><i class="fa fa-fast-forward"></i></button>
{{#if displayHide}}
<button title="{{i18n post.revisions.controls.hide}}" {{bind-attr class=":btn :standard :no-text" disabled=loading}} {{action "hideVersion"}}><i class="fa fa-trash-o"></i></button>
<button title="{{i18n post.revisions.controls.hide}}" {{bind-attr class=":btn :standard :no-text :btn-danger" disabled=loading}} {{action "hideVersion"}}><i class="fa fa-trash-o"></i></button>
{{/if}}
{{#if displayShow}}
<button title="{{i18n post.revisions.controls.show}}" {{bind-attr class=":btn :standard :no-text" disabled=loading}} {{action "showVersion"}}><i class="fa fa-undo"></i></button>
@ -28,34 +28,61 @@
</div>
</div>
<div id="revision-details">
{{i18n post.revisions.details.edited_by}} {{#link-to 'user' username}}{{bound-avatar-template content.avatar_template "small"}} {{username}}{{/link-to}} <span class="date">{{bound-date created_at}}</span> {{#if edit_reason}} &mdash; <span class="edit-reason">{{edit_reason}}</span>{{/if}}
{{i18n post.revisions.details.edited_by}}
{{#link-to 'user' username}}
{{bound-avatar-template content.avatar_template "small"}} {{username}}
{{/link-to}}
<span class="date">{{bound-date created_at}}</span>
{{#if edit_reason}}
&mdash; <span class="edit-reason">{{edit_reason}}</span>
{{/if}}
{{#unless site.mobileView}}
{{#if user_changes}}
&mdash; {{bound-avatar-template user_changes.previous.avatar_template "small"}} {{user_changes.previous.username}}
&rarr; {{bound-avatar-template user_changes.current.avatar_template "small"}} {{user_changes.current.username}}
{{/if}}
{{#if wiki_changes}}
&mdash; {{{wiki_diff}}}
{{/if}}
{{#if post_type_changes}}
&mdash; {{{post_type_diff}}}
{{/if}}
{{#if category_changes}}
&mdash; {{{previousCategory}}} &rarr; {{{currentCategory}}}
{{/if}}
{{/unless}}
</div>
<div id="revisions" {{bind-attr class="hidden:hidden-revision"}}>
<div id="revisions" {{bind-attr class="hiddenClasses"}}>
{{#if title_changes}}
<div class="row">
<h2>{{{title_diff}}}</h2>
</div>
{{/if}}
{{#if category_changes}}
<div class="row">
{{{category_diff}}}
</div>
{{#if site.mobileView}}
{{#if user_changes}}
<div class="row">
{{bound-avatar-template user_changes.previous.avatar_template "small"}} {{user_changes.previous.username}}
&rarr; {{bound-avatar-template user_changes.current.avatar_template "small"}} {{user_changes.current.username}}
</div>
{{/if}}
{{#if wiki_changes}}
<div class="row">
{{{wiki_diff}}}
</div>
{{/if}}
{{#if post_type_changes}}
<div class="row">
{{{post_type_diff}}}
</div>
{{/if}}
{{#if category_changes}}
<div class="row">
{{{previousCategory}}} &rarr; {{{currentCategory}}}
</div>
{{/if}}
{{/if}}
{{#if user_changes}}
<div class="row">
{{bound-avatar-template user_changes.previous.avatar_template "small"}} {{user_changes.previous.username}} → {{bound-avatar-template user_changes.current.avatar_template "small"}} {{user_changes.current.username}}
</div>
{{/if}}
{{#if wiki_changes}}
<div class="row">
{{{wiki_diff}}}
</div>
{{/if}}
{{#if post_type_changes}}
<div class="row">
{{{post_type_diff}}}
</div>
{{/if}}
{{{body_diff}}}
<div class="row">
{{{body_diff}}}
</div>
</div>
</div>

17
app/assets/stylesheets/common/base/history.scss
View File

@ -11,6 +11,9 @@
margin-right: 7px;
}
}
#revisions .row:first-of-type {
margin-top: 10px;
}
ins, .diff-ins {
code, img {
border: 2px solid $success;
@ -75,7 +78,17 @@
.fa-ban {
color: #f00;
}
.hidden-revision {
opacity: 0.5;
.hidden-revision-either {
opacity: .5;
}
.hidden-revision-previous .row {
div:nth-of-type(1), td:nth-of-type(1) {
opacity: .5;
}
}
.hidden-revision-current .row {
div:nth-of-type(2), td:nth-of-type(2) {
opacity: .5;
}
}
}

10
app/assets/stylesheets/desktop/history.scss
View File

@ -1,6 +1,10 @@
// styles that apply to the popup that appears when you show the edit history of a post
.modal.history-modal {
.btn {
// remove transitions on the buttons in the history modal
transition: none;
}
.modal-inner-container {
min-width: 960px;
min-height: 500px;
@ -18,13 +22,13 @@
background-color: scale-color-diff();
padding: 5px;
margin-top: 10px;
line-height: 2em;
height: 30px;
}
#revisions {
word-wrap: break-word;
.row, table {
table {
margin-top: 10px;
padding-bottom: 10px;
border-bottom: 1px dotted;
}
}
img {

2
app/assets/stylesheets/mobile/modal.scss
View File

@ -35,7 +35,7 @@
.modal-body {
overflow-y: auto;
max-height: 400px;
padding: 10px 0 10px 15px;
padding: 10px 0 10px 10px;
width: 95%;
}

85
app/controllers/posts_controller.rb
View File

@ -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, :reply_history, :revisions, :expand_embed, :markdown, :raw, :cooked]
before_filter :ensure_logged_in, except: [:show, :replies, :by_number, :short_link, :reply_history, :revisions, :latest_revision, :expand_embed, :markdown, :raw, :cooked]
skip_before_filter :check_xhr, only: [:markdown_id, :markdown_num, :short_link]
@ -99,39 +99,30 @@ class PostsController < ApplicationController
post.image_sizes = params[:image_sizes] if params[:image_sizes].present?
if too_late_to(:edit, post)
render json: {errors: [I18n.t('too_late_to_edit')]}, status: 422
return
return render json: { errors: [I18n.t('too_late_to_edit')] }, status: 422
end
guardian.ensure_can_edit!(post)
# to stay consistent with the create api,
# 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_id])
post.topic.acting_user = current_user
post.topic.title = params[:title] if params[:title]
Topic.transaction do
post.topic.change_category_to_id(params[:post][:category_id].to_i)
post.topic.save
end
changes = {
raw: params[:post][:raw],
edit_reason: params[:post][:edit_reason]
}
if post.topic.errors.present?
render_json_error(post.topic)
return
end
# to stay consistent with the create api, we allow for title & category changes here
if post.post_number == 1
changes[:title] = params[:title] if params[:title]
changes[:category_id] = params[:post][:category_id] if params[:post][:category_id]
end
revisor = PostRevisor.new(post)
if revisor.revise!(current_user, params[:post][:raw], edit_reason: params[:post][:edit_reason])
if revisor.revise!(current_user, changes)
TopicLink.extract_from(post)
QuotedPost.extract_from(post)
end
if post.errors.present?
render_json_error(post)
return
end
return render_json_error(post) if post.errors.present?
return render_json_error(post.topic) if post.topic.errors.present?
post_serializer = PostSerializer.new(post, scope: guardian, root: false)
post_serializer.draft_sequence = DraftSequence.current(current_user, post.topic.draft_key)
@ -194,7 +185,6 @@ class PostsController < ApplicationController
end
def destroy_many
params.require(:post_ids)
posts = Post.where(id: post_ids_including_replies)
@ -222,17 +212,35 @@ class PostsController < ApplicationController
render_json_dump(post_revision_serializer)
end
def latest_revision
post_revision = find_latest_post_revision_from_params
post_revision_serializer = PostRevisionSerializer.new(post_revision, scope: guardian, root: false)
render_json_dump(post_revision_serializer)
end
def hide_revision
post_revision = find_post_revision_from_params
guardian.ensure_can_hide_post_revision! post_revision
guardian.ensure_can_hide_post_revision!(post_revision)
post_revision.hide!
post = find_post_from_params
post.public_version -= 1
post.save
render nothing: true
end
def show_revision
post_revision = find_post_revision_from_params
guardian.ensure_can_show_post_revision! post_revision
guardian.ensure_can_show_post_revision!(post_revision)
post_revision.show!
post = find_post_from_params
post.public_version += 1
post.save
render nothing: true
end
@ -252,9 +260,7 @@ class PostsController < ApplicationController
guardian.ensure_can_wiki!
post = find_post_from_params
post.wiki = params[:wiki]
post.version += 1
post.save
post.revise(current_user, { wiki: params[:wiki] })
render nothing: true
end
@ -263,9 +269,7 @@ class PostsController < ApplicationController
guardian.ensure_can_change_post_type!
post = find_post_from_params
post.post_type = params[:post_type].to_i
post.version += 1
post.save
post.revise(current_user, { post_type: params[:post_type].to_i })
render nothing: true
end
@ -329,9 +333,26 @@ class PostsController < ApplicationController
raise Discourse::InvalidParameters.new(:revision) if revision < 2
post_revision = PostRevision.find_by(post_id: post_id, number: revision)
post_revision.post = find_post_from_params
raise Discourse::NotFound unless post_revision
post_revision.post = find_post_from_params
guardian.ensure_can_see!(post_revision)
post_revision
end
def find_latest_post_revision_from_params
post_id = params[:id] || params[:post_id]
finder = PostRevision.where(post_id: post_id).order(:number)
finder = finder.where(hidden: false) unless guardian.is_staff?
post_revision = finder.last
raise Discourse::NotFound unless post_revision
post_revision.post = find_post_from_params
guardian.ensure_can_see!(post_revision)
post_revision
end

35
app/controllers/topics_controller.rb
View File

@ -122,14 +122,15 @@ class TopicsController < ApplicationController
topic = Topic.find_by(id: params[:topic_id])
guardian.ensure_can_edit!(topic)
topic.title = params[:title] if params[:title].present?
topic.acting_user = current_user
changes = {}
changes[:title] = params[:title] if params[:title]
changes[:category_id] = params[:category_id] if params[:category_id]
success = false
Topic.transaction do
success = topic.save
success &= topic.change_category_to_id(params[:category_id].to_i) unless topic.private_message?
EditRateLimiter.new(current_user).performed!
success = true
if changes.length > 0
first_post = topic.ordered_posts.first
success = PostRevisor.new(first_post, topic).revise!(current_user, changes)
end
# this is used to return the title to the client as it may have been changed by "TextCleaner"
@ -308,21 +309,17 @@ class TopicsController < ApplicationController
guardian.ensure_can_change_post_owner!
topic = Topic.find(params[:topic_id].to_i)
new_user = User.find_by_username(params[:username])
ids = params[:post_ids].to_a
post_ids = params[:post_ids].to_a
topic = Topic.find_by(id: params[:topic_id].to_i)
new_user = User.find_by(username: params[:username])
unless new_user && topic && ids
render json: failed_json, status: 422
return
end
return render json: failed_json, status: 422 unless post_ids && topic && new_user
ActiveRecord::Base.transaction do
ids.each do |id|
post = Post.find(id)
if post.is_first_post?
topic.user = new_user # Update topic owner (first avatar)
end
post_ids.each do |post_id|
post = Post.find(post_id)
# update topic owner (first avatar)
topic.user = new_user if post.is_first_post?
post.set_owner(new_user, current_user)
end
end

9
app/jobs/regular/pull_hotlinked_images.rb
View File

@ -84,11 +84,10 @@ module Jobs
delay = SiteSetting.ninja_edit_window * args[:backoff]
Jobs.enqueue_in(delay.seconds.to_i, :pull_hotlinked_images, args.merge!(backoff: backoff))
elsif raw != post.raw
options = {
edit_reason: I18n.t("upload.edit_reason"),
bypass_bump: true # we never want that job to bump the topic
}
post.revise(Discourse.system_user, raw, options)
changes = { raw: raw, edit_reason: I18n.t("upload.edit_reason") }
# we never want that job to bump the topic
options = { bypass_bump: true }
post.revise(Discourse.system_user, changes, options)
end
end

54
app/models/post.rb
View File

@ -324,8 +324,8 @@ class Post < ActiveRecord::Base
end
end
def revise(updated_by, new_raw, opts = {})
PostRevisor.new(self).revise!(updated_by, new_raw, opts)
def revise(updated_by, changes={}, opts={})
PostRevisor.new(self).revise!(updated_by, changes, opts)
end
def self.rebake_old(limit)
@ -364,13 +364,14 @@ class Post < ActiveRecord::Base
end
def set_owner(new_user, actor)
revise(actor, self.raw, {
new_user: new_user,
changed_owner: true,
edit_reason: I18n.t('change_owner.post_revision_text',
old_user: self.user.username_lower,
new_user: new_user.username_lower)
})
return if user_id == new_user.id
edit_reason = I18n.t('change_owner.post_revision_text',
old_user: self.user.username_lower,
new_user: new_user.username_lower
)
revise(actor, { raw: self.raw, user_id: new_user.id, edit_reason: edit_reason })
end
before_create do
@ -414,14 +415,6 @@ class Post < ActiveRecord::Base
self.baked_version = BAKED_VERSION
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)
@ -537,33 +530,6 @@ class Post < ActiveRecord::Base
end
end
def save_revision
modifications = changes.extract!(:raw, :cooked, :edit_reason, :user_id, :wiki, :post_type)
# 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.find_by(post_id: id, number: version)
return unless revision
revision.user_id = last_editor_id
modifications = changes.extract!(:raw, :cooked, :edit_reason)
[:raw, :cooked, :edit_reason].each do |field|
if modifications[field].present?
old_value = revision.modifications[field].try(:[], 0) || ""
new_value = modifications[field][1]
revision.modifications[field] = [old_value, new_value]
end
end
revision.save
end
end
# == Schema Information

4
app/models/post_action.rb
View File

@ -163,9 +163,7 @@ class PostAction < ActiveRecord::Base
end
def moderator_already_replied?(topic, moderator)
topic.posts
.where("user_id = :user_id OR post_type = :post_type", user_id: moderator.id, post_type: Post.types[:moderator_action])
.exists?
topic.posts.where("user_id = :user_id OR post_type = :post_type", user_id: moderator.id, post_type: Post.types[:moderator_action]).exists?
end
def self.create_message_for_post_action(user, post, post_action_type_id, opts)

17
app/models/post_alert_observer.rb
View File

@ -30,11 +30,13 @@ class PostAlertObserver < ActiveRecord::Observer
post = post_action.post
return if post_action.user.blank?
alerter.create_notification(post.user,
Notification.types[:liked],
post,
display_username: post_action.user.username,
post_action_id: post_action.id)
alerter.create_notification(
post.user,
Notification.types[:liked],
post,
display_username: post_action.user.username,
post_action_id: post_action.id
)
end
def after_create_post_revision(post_revision)
@ -50,12 +52,11 @@ class PostAlertObserver < ActiveRecord::Observer
post.user,
Notification.types[:edited],
post,
display_username: post_revision.user.username,
post_revision: post_revision
display_username: post_revision.user.username,
acting_user_id: post_revision.try(:user_id)
)
end
protected
def callback_for(action, model)

173
app/models/post_revision.rb
View File

@ -8,185 +8,30 @@ class PostRevision < ActiveRecord::Base
def self.ensure_consistency!
# 1 - fix the numbers
sql = <<-SQL
PostRevision.exec_sql <<-SQL
UPDATE post_revisions
SET number = pr.rank
FROM (SELECT id, ROW_NUMBER() OVER (PARTITION BY post_id ORDER BY number, created_at, updated_at) AS rank FROM post_revisions) AS pr
FROM (SELECT id, 1 + ROW_NUMBER() OVER (PARTITION BY post_id ORDER BY number, created_at, updated_at) AS rank FROM post_revisions) AS pr
WHERE post_revisions.id = pr.id
AND post_revisions.number <> pr.rank
SQL
PostRevision.exec_sql(sql)
# 2 - fix the versions on the posts
sql = <<-SQL
PostRevision.exec_sql <<-SQL
UPDATE posts
SET version = pv.version
FROM (SELECT post_id, MAX(number) AS version FROM post_revisions GROUP BY post_id) AS pv
WHERE posts.id = pv.post_id
AND posts.version <> pv.version
SET version = 1 + (SELECT COUNT(*) FROM post_revisions WHERE post_id = posts.id),
public_version = 1 + (SELECT COUNT(*) FROM post_revisions pr WHERE post_id = posts.id AND pr.hidden = 'f')
WHERE version <> 1 + (SELECT COUNT(*) FROM post_revisions WHERE post_id = posts.id)
OR public_version <> 1 + (SELECT COUNT(*) FROM post_revisions pr WHERE post_id = posts.id AND pr.hidden = 'f')
SQL
PostRevision.exec_sql(sql)
end
def body_changes
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: prev,
current_category_id: cur,
}
end
def wiki_changes
prev = previous("wiki")
cur = current("wiki")
return if prev == cur
{
previous_wiki: prev,
current_wiki: cur,
}
end
def post_type_changes
prev = previous("post_type")
cur = current("post_type")
return if prev == cur
{
previous_post_type: prev,
current_post_type: cur,
}
end
def title_changes
prev = "<div>#{CGI::escapeHTML(previous("title"))}</div>"
cur = "<div>#{CGI::escapeHTML(current("title"))}</div>"
return if prev == cur
diff = DiscourseDiff.new(prev, cur)
{
inline: diff.inline_html,
side_by_side: diff.side_by_side_html
}
end
def user_changes
prev = previous("user_id")
cur = current("user_id")
return if prev == cur
{
previous_user: User.find_by(id: prev),
current_user: User.find_by(id: cur)
}
end
def previous(field)
val = lookup(field)
if val.nil?
val = lookup_in_previous_revisions(field)
end
if val.nil?
val = lookup_in_post(field)
end
val
end
def current(field)
val = lookup_in_next_revision(field)
if val.nil?
val = lookup_in_post(field)
end
if val.nil?
val = lookup(field)
end
if val.nil?
val = lookup_in_previous_revisions(field)
end
return val
end
def previous_revisions
@previous_revs ||= PostRevision.where("post_id = ? AND number < ? AND hidden = ?", post_id, number, false)
.order("number desc")
.to_a
end
def next_revision
@next_revision ||= PostRevision.where("post_id = ? AND number > ? AND hidden = ?", post_id, number, false)
.order("number asc")
.to_a.first
end
def has_topic_data?
post && post.post_number == 1
end
def lookup_in_previous_revisions(field)
previous_revisions.each do |v|
val = v.lookup(field)
return val unless val.nil?
end
nil
end
def lookup_in_next_revision(field)
if next_revision
return next_revision.lookup(field)
end
end
def lookup_in_post(field)
if !post
return
elsif ["cooked", "raw"].include?(field)
val = post.send(field)
elsif ["title", "category_id"].include?(field)
val = post.topic.send(field)
end
val
end
def lookup(field)
return nil if hidden
mod = modifications[field]
unless mod.nil?
mod[0]
end
end
def hide!
self.hidden = true
self.save!
update_column(:hidden, true)
end
def show!
self.hidden = false
self.save!
update_column(:hidden, false)
end
end

133
app/models/topic.rb
View File

@ -99,7 +99,6 @@ class Topic < ActiveRecord::Base
has_many :topic_invites
has_many :invites, through: :topic_invites, source: :invite
has_many :revisions, foreign_key: :topic_id, class_name: 'TopicRevision'
has_one :warning
# When we want to temporarily attach some data to a forum topic (usually before serialization)
@ -148,70 +147,71 @@ class Topic < ActiveRecord::Base
end
attr_accessor :ignore_category_auto_close
attr_accessor :skip_callbacks
before_create do
self.bumped_at ||= Time.now
self.last_post_user_id ||= user_id
if !@ignore_category_auto_close and self.category and self.category.auto_close_hours and self.auto_close_at.nil?
self.auto_close_based_on_last_post = self.category.auto_close_based_on_last_post
set_auto_close(self.category.auto_close_hours)
end
initialize_default_values
inherit_auto_close_from_category
end
attr_accessor :skip_callbacks
after_create do
unless skip_callbacks
changed_to_category(category)
if archetype == Archetype.private_message
DraftSequence.next!(user, Draft::NEW_PRIVATE_MESSAGE)
else
DraftSequence.next!(user, Draft::NEW_TOPIC)
end
advance_draft_sequence
end
end
before_save do
unless skip_callbacks
if (auto_close_at_changed? and !auto_close_at_was.nil?) or (auto_close_user_id_changed? and auto_close_at)
self.auto_close_started_at ||= Time.zone.now if auto_close_at
Jobs.cancel_scheduled_job(:close_topic, {topic_id: id})
end
if category_id.nil? && (archetype.nil? || archetype == Archetype.default)
self.category_id = SiteSetting.uncategorized_category_id
end
cancel_auto_close_job
ensure_topic_has_a_category
end
end
after_save do
save_revision if should_create_new_version?
unless skip_callbacks
if auto_close_at and (auto_close_at_changed? or auto_close_user_id_changed?)
Jobs.enqueue_at(auto_close_at, :close_topic, {topic_id: id, user_id: auto_close_user_id || user_id})
end
schedule_auto_close_job
end
end
# TODO move into PostRevisor or TopicRevisor
def save_revision
if first_post_id = posts.where(post_number: 1).pluck(:id).first
def initialize_default_values
self.bumped_at ||= Time.now
self.last_post_user_id ||= user_id
end
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.where(id: first_post_id).update_all(version: number)
def inherit_auto_close_from_category
if !@ignore_category_auto_close && self.category && self.category.auto_close_hours && self.auto_close_at.nil?
self.auto_close_based_on_last_post = self.category.auto_close_based_on_last_post
set_auto_close(self.category.auto_close_hours)
end
end
def should_create_new_version?
!new_record? && (category_id_changed? || title_changed?)
def advance_draft_sequence
if archetype == Archetype.private_message
DraftSequence.next!(user, Draft::NEW_PRIVATE_MESSAGE)
else
DraftSequence.next!(user, Draft::NEW_TOPIC)
end
end
def cancel_auto_close_job
if (auto_close_at_changed? && !auto_close_at_was.nil?) || (auto_close_user_id_changed? && auto_close_at)
self.auto_close_started_at ||= Time.zone.now if auto_close_at
Jobs.cancel_scheduled_job(:close_topic, { topic_id: id })
end
end
def schedule_auto_close_job
if auto_close_at && (auto_close_at_changed? || auto_close_user_id_changed?)
options = { topic_id: id, user_id: auto_close_user_id || user_id }
Jobs.enqueue_at(auto_close_at, :close_topic, options)
end
end
def ensure_topic_has_a_category
if category_id.nil? && (archetype.nil? || archetype == Archetype.default)
self.category_id = SiteSetting.uncategorized_category_id
end
end
def self.top_viewed(max = 10)
@ -266,10 +266,6 @@ class Topic < ActiveRecord::Base
Redcarpet::Render::SmartyPants.render(sanitized_title)
end
def new_version_required?
title_changed? || category_id_changed?
end
# Returns hot topics since a date for display in email digest.
def self.for_digest(user, since, opts=nil)
opts = opts || {}
@ -383,10 +379,8 @@ class Topic < ActiveRecord::Base
candidate_ids = candidates.pluck(:id)
return [] unless candidate_ids.present?
similar = Topic.select(sanitize_sql_array(["topics.*, similarity(topics.title, :title) + similarity(topics.title, :raw) AS similarity", title: title, raw: raw]))
.joins("JOIN posts AS p ON p.topic_id = topics.id AND p.post_number = 1")
.limit(SiteSetting.max_similar_results)
@ -451,37 +445,30 @@ class Topic < ActiveRecord::Base
(topics.avg_time <> x.gmean OR topics.avg_time IS NULL)")
if min_topic_age
builder.where("topics.bumped_at > :bumped_at",
bumped_at: min_topic_age)
builder.where("topics.bumped_at > :bumped_at", bumped_at: min_topic_age)
end
builder.exec
end
def changed_to_category(cat)
return true if cat.blank? || Category.find_by(topic_id: id).present?
def changed_to_category(new_category)
return true if new_category.blank? || Category.find_by(topic_id: id).present?
return false if new_category.id == SiteSetting.uncategorized_category_id && !SiteSetting.allow_uncategorized_topics
Topic.transaction do
old_category = category
if category_id.present? && category_id != cat.id
Category.where(['id = ?', category_id]).update_all 'topic_count = topic_count - 1'
if self.category_id != new_category.id
self.category_id = new_category.id
self.update_column(:category_id, new_category.id)
Category.where(id: old_category.id).update_all("topic_count = topic_count - 1") if old_category
end
success = true
if self.category_id != cat.id
self.category_id = cat.id
success = save
end
if success
CategoryFeaturedTopic.feature_topics_for(old_category) unless @import_mode
Category.where(id: cat.id).update_all 'topic_count = topic_count + 1'
CategoryFeaturedTopic.feature_topics_for(cat) unless @import_mode || old_category.try(:id) == cat.try(:id)
else
return false
end
Category.where(id: new_category.id).update_all("topic_count = topic_count + 1")
CategoryFeaturedTopic.feature_topics_for(old_category) unless @import_mode
CategoryFeaturedTopic.feature_topics_for(new_category) unless @import_mode || old_category.id == new_category.id
end
true
end
@ -513,15 +500,15 @@ class Topic < ActiveRecord::Base
def change_category_to_id(category_id)
return false if private_message?
# If the category name is blank, reset the attribute
if (category_id.nil? || category_id.to_i == 0)
cat = Category.find_by(id: SiteSetting.uncategorized_category_id)
else
cat = Category.where(id: category_id).first
end
new_category_id = category_id.to_i
# if the category name is blank, reset the attribute
new_category_id = SiteSetting.uncategorized_category_id if new_category_id == 0
return true if cat == category
return true if self.category_id == new_category_id
cat = Category.find_by(id: new_category_id)
return false unless cat
changed_to_category(cat)
end

3
app/models/topic_embed.rb
View File

@ -50,8 +50,7 @@ class TopicEmbed < ActiveRecord::Base
post = embed.post
# Update the topic if it changed
if post && post.topic && content_sha1 != embed.content_sha1
revisor = PostRevisor.new(post)
revisor.revise!(user, absolutize_urls(url, contents), skip_validations: true, bypass_rate_limiter: true)
post.revise(user, { raw: absolutize_urls(url, contents) }, skip_validations: true, bypass_rate_limiter: true)
embed.update_column(:content_sha1, content_sha1)
end
end

2
app/models/user_action_observer.rb
View File

@ -33,7 +33,7 @@ class UserActionObserver < ActiveRecord::Observer
end
end
def self.log_notification(post, user, notification_type, acting_user_id = nil)
def self.log_notification(post, user, notification_type, acting_user_id=nil)
action =
case notification_type
when Notification.types[:quoted]

223
app/serializers/post_revision_serializer.rb
View File

@ -1,38 +1,70 @@
class PostRevisionSerializer < ApplicationSerializer
attributes :post_id,
:version,
:revisions_count,
attributes :created_at,
:post_id,
# which revision is hidden
:previous_hidden,
:current_hidden,
# dynamic & based on the current scope
:first_revision,
:previous_revision,
:current_revision,
:next_revision,
:last_revision,
# used for display
:current_version,
:version_count,
# from the user
:username,
:display_username,
:avatar_template,
:created_at,
# all the changes
:edit_reason,
:body_changes,
:title_changes,
:category_changes,
:user_changes,
:wiki_changes,
:post_type_changes,
:hidden
:post_type_changes
def include_title_changes?
object.has_topic_data?
def previous_hidden
previous["hidden"]
end
def include_category_changes?
object.has_topic_data?
def current_hidden
current["hidden"]
end
def hidden
object.hidden
def first_revision
revisions.first["revision"]
end
def version
def previous_revision
@previous_revision ||= revisions.select { |r| r["revision"] >= first_revision }
.select { |r| r["revision"] < current_revision }
.last.try(:[], "revision")
end
def current_revision
object.number
end
def revisions_count
object.post.version
def next_revision
@next_revision ||= revisions.select { |r| r["revision"] <= last_revision }
.select { |r| r["revision"] > current_revision }
.first.try(:[], "revision")
end
def last_revision
@last_revision ||= revisions.select { |r| r["revision"] <= post.version }.last["revision"]
end
def current_version
@current_version ||= revisions.select { |r| r["revision"] <= current_revision }.count + 1
end
def version_count
revisions.count
end
def username
@ -48,32 +80,163 @@ class PostRevisionSerializer < ApplicationSerializer
end
def edit_reason
object.current("edit_reason")
# only show 'edit_reason' when revisions are consecutive
current["edit_reason"] if scope.can_view_hidden_post_revisions? ||
current["revision"] == previous["revision"] + 1
end
def body_changes
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 title_changes
prev = "<div>#{CGI::escapeHTML(previous["title"])}</div>"
cur = "<div>#{CGI::escapeHTML(current["title"])}</div>"
return if prev == cur
diff = DiscourseDiff.new(prev, cur)
{
inline: diff.inline_html,
side_by_side: diff.side_by_side_html
}
end
def category_changes
prev = previous["category_id"]
cur = current["category_id"]
return if prev == cur
{
previous: prev,
current: cur,
}
end
def user_changes
obj = object.user_changes
return unless obj
# same as below - if stuff is messed up, default to system
prev = obj[:previous_user] || Discourse.system_user
new = obj[:current_user] || Discourse.system_user
prev = previous["user_id"]
cur = current["user_id"]
return if prev == cur
# if stuff is messed up, default to system
previous = User.find_by(id: prev) || Discourse.system_user
current = User.find_by(id: cur) || Discourse.system_user
{
previous: {
username: prev.username_lower,
display_username: prev.username,
avatar_template: prev.avatar_template
username: previous.username_lower,
display_username: previous.username,
avatar_template: previous.avatar_template
},
current: {
username: new.username_lower,
display_username: new.username,
avatar_template: new.avatar_template
username: current.username_lower,
display_username: current.username,
avatar_template: current.avatar_template
}
}
end
def user
# if stuff goes pear shape attribute to system
object.user || Discourse.system_user
def wiki_changes
prev = previous["wiki"]
cur = current["wiki"]
return if prev == cur
{
previous: prev,
current: cur,
}
end
def post_type_changes
prev = previous["post_type"]
cur = current["post_type"]
return if prev == cur
{
previous: prev,
current: cur,
}
end
protected
def post
@post ||= object.post
end
def topic
@topic ||= object.post.topic
end
def revisions
@revisions ||= all_revisions.select { |r| scope.can_view_hidden_post_revisions? || !r["hidden"] }
end
def all_revisions
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: {
"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],
}
)
@all_revisions = []
# backtrack
post_revisions.each do |pr|
revision = HashWithIndifferentAccess.new
revision[:revision] = pr.number
revision[:hidden] = pr.hidden
pr.modifications.keys.each do |field|
revision[field] = pr.modifications[field][0]
end
@all_revisions << revision
end
# waterfall
(@all_revisions.count - 1).downto(1).each do |r|
cur = @all_revisions[r]
prev = @all_revisions[r - 1]
cur.keys.each do |field|
prev[field] = prev.has_key?(field) ? prev[field] : cur[field]
end
end
@all_revisions
end
def previous
@previous ||= revisions.select { |r| r["revision"] <= current_revision }.last
end
def current
@current ||= revisions.select { |r| r["revision"] > current_revision }.first
end
def user
# if stuff goes pear shape attribute to system
object.user || Discourse.system_user
end
end

6
app/serializers/post_serializer.rb
View File

@ -233,7 +233,7 @@ class PostSerializer < BasicPostSerializer
end
def can_view_edit_history
scope.can_view_post_revisions?(object)
scope.can_view_edit_history?(object)
end
def user_custom_fields
@ -258,6 +258,10 @@ class PostSerializer < BasicPostSerializer
object.via_email?
end
def version
scope.is_staff? ? object.version : object.public_version
end
private
def post_actions

10
app/services/post_alerter.rb
View File

@ -94,22 +94,20 @@ class PostAlerter
.order("notifications.id desc")
.find_by(notification_type: type, topic_id: post.topic_id, post_number: post.post_number)
if(existing_notification)
if existing_notification
return unless existing_notification.notification_type == Notification.types[:edited] &&
existing_notification.data_hash["display_username"] = opts[:display_username]
end
collapsed = false
if type == Notification.types[:replied] ||
type == Notification.types[:posted]
if type == Notification.types[:replied] || type == Notification.types[:posted]
destroy_notifications(user, Notification.types[:replied] , post.topic)
destroy_notifications(user, Notification.types[:posted] , post.topic)
collapsed = true
end
if type == Notification.types[:private_message]
if type == Notification.types[:private_message]
destroy_notifications(user, type, post.topic)
collapsed = true
end
@ -123,7 +121,7 @@ class PostAlerter
opts[:display_username] = I18n.t('embed.replies', count: count) if count > 1
end
UserActionObserver.log_notification(original_post, user, type, opts[:post_revision].try(:user_id))
UserActionObserver.log_notification(original_post, user, type, opts[:acting_user_id])
# Create the notification
user.notifications.create(notification_type: type,

7
config/routes.rb
View File

@ -282,9 +282,10 @@ Discourse::Application.routes.draw do
put "rebake"
put "unhide"
get "replies"
get "revisions/:revision" => "posts#revisions"
put "revisions/:revision/hide" => "posts#hide_revision"
put "revisions/:revision/show" => "posts#show_revision"
get "revisions/latest" => "posts#latest_revision"
get "revisions/:revision" => "posts#revisions", constraints: { revision: /\d+/ }
put "revisions/:revision/hide" => "posts#hide_revision", constraints: { revision: /\d+/ }
put "revisions/:revision/show" => "posts#show_revision", constraints: { revision: /\d+/ }
put "recover"
collection do
delete "destroy_many"

15
db/migrate/20141020153415_add_public_version_to_posts.rb Normal file
View File

@ -0,0 +1,15 @@
class AddPublicVersionToPosts < ActiveRecord::Migration
def up
add_column :posts, :public_version, :integer, null: false, default: 1
execute <<-SQL
UPDATE posts
SET public_version = 1 + (SELECT COUNT(*) FROM post_revisions pr WHERE post_id = posts.id AND pr.hidden = 'f')
WHERE public_version <> 1 + (SELECT COUNT(*) FROM post_revisions pr WHERE post_id = posts.id AND pr.hidden = 'f')
SQL
end
def down
remove_column :posts, :public_version
end
end

47
lib/discourse_observer.rb
View File

@ -1,47 +0,0 @@
#
# Support delegating after_create to an appropriate helper for that class name.
# For example, an observer on post will call after_create_post if that method
# is defined.
#
# It does this after_commit by default, and contains a hack to make this work
# even in test mode.
#
class DiscourseObserver < ActiveRecord::Observer
def after_create_delegator(model)
observer_method = :"after_create_#{model.class.name.underscore}"
send(observer_method, model) if respond_to?(observer_method)
end
def after_destroy_delegator(model)
observer_method = :"after_destroy_#{model.class.name.underscore}"
send(observer_method, model) if respond_to?(observer_method)
end
end
if Rails.env.test?
# In test mode, call the delegator right away
class DiscourseObserver < ActiveRecord::Observer
alias_method :after_create, :after_create_delegator
alias_method :after_destroy, :after_destroy_delegator
end
else
# Outside of test mode, use after_commit
class DiscourseObserver < ActiveRecord::Observer
def after_commit(model)
if model.send(:transaction_include_any_action?, [:create])
after_create_delegator(model)
end
if model.send(:transaction_include_any_action?, [:destroy])
after_destroy_delegator(model)
end
end
end
end

2
lib/guardian.rb
View File

@ -3,6 +3,7 @@ require_dependency 'guardian/ensure_magic'
require_dependency 'guardian/post_guardian'
require_dependency 'guardian/topic_guardian'
require_dependency 'guardian/user_guardian'
require_dependency 'guardian/post_revision_guardian'
# The guardian is responsible for confirming access to various site resources and operations
class Guardian
@ -11,6 +12,7 @@ class Guardian
include PostGuardian
include TopicGuardian
include UserGuardian
include PostRevisionGuardian
class AnonymousUser
def blank?; true; end

15
lib/guardian/post_guardian.rb
View File

@ -140,12 +140,7 @@ module PostGuardian
can_see_topic?(post.topic)))
end
def can_see_post_revision?(post_revision)
return false unless post_revision
can_view_post_revisions?(post_revision.post)
end
def can_view_post_revisions?(post)
def can_view_edit_history?(post)
return false unless post
if !post.hidden
@ -157,14 +152,6 @@ module PostGuardian
can_see_post?(post)
end
def can_hide_post_revision?(post_revision)
is_staff?
end
def can_show_post_revision?(post_revision)
is_staff?
end
def can_vote?(post, opts={})
post_can_act?(post,:vote, opts)
end

23
lib/guardian/post_revision_guardian.rb Normal file
View File

@ -0,0 +1,23 @@
# mixin for all Guardian methods dealing with post_revisions permissions
module PostRevisionGuardian
def can_see_post_revision?(post_revision)
return false unless post_revision
return false if post_revision.hidden && !can_view_hidden_post_revisions?
can_view_edit_history?(post_revision.post)
end
def can_hide_post_revision?(post_revision)
is_staff?
end
def can_show_post_revision?(post_revision)
is_staff?
end
def can_view_hidden_post_revisions?
is_staff?
end
end

4
lib/post_destroyer.rb
View File

@ -96,7 +96,7 @@ class PostDestroyer
# When a user 'deletes' their own post. We just change the text.
def mark_for_deletion
Post.transaction do
@post.revise(@user, I18n.t('js.post.deleted_by_author', count: SiteSetting.delete_removed_posts_after), force_new_version: true)
@post.revise(@user, { raw: I18n.t('js.post.deleted_by_author', count: SiteSetting.delete_removed_posts_after) }, force_new_version: true)
@post.update_column(:user_deleted, true)
@post.update_flagged_posts_count
@post.topic_links.each(&:destroy)
@ -110,7 +110,7 @@ class PostDestroyer
Post.transaction do
@post.update_column(:user_deleted, false)
@post.skip_unique_check = true
@post.revise(@user, @post.revisions.last.modifications["raw"][0], force_new_version: true)
@post.revise(@user, { raw: @post.revisions.last.modifications["raw"][0] }, force_new_version: true)
@post.update_flagged_posts_count
end

314
lib/post_revisor.rb
View File

@ -1,138 +1,269 @@
require 'edit_rate_limiter'
require "edit_rate_limiter"
class PostRevisor
POST_TRACKED_FIELDS = %w{raw cooked edit_reason user_id wiki post_type}
TOPIC_TRACKED_FIELDS = %w{title category_id}
attr_reader :category_changed
def initialize(post)
def initialize(post, topic=nil)
@post = post
@topic = topic || post.topic
end
# Recognized options:
# :edit_reason User-supplied edit reason
# :new_user New owner of the post
# :revised_at changes the date of the revision
# :force_new_version bypass ninja-edit window
# :bypass_bump do not bump the topic, even if last post
# :skip_validation ask ActiveRecord to skip validations
#
def revise!(editor, new_raw, opts = {})
# AVAILABLE OPTIONS:
# - revised_at: changes the date of the revision
# - force_new_version: bypass ninja-edit window
# - bypass_rate_limiter:
# - bypass_bump: do not bump the topic, even if last post
# - skip_validations: ask ActiveRecord to skip validations
def revise!(editor, fields, opts={})
@editor = editor
@fields = fields.with_indifferent_access
@opts = opts
@new_raw = TextCleaner.normalize_whitespaces(new_raw).gsub(/\s+\z/, "")
# some normalization
@fields[:raw] = cleanup_whitespaces(@fields[:raw]) if @fields.has_key?(:raw)
@fields[:user_id] = @fields[:user_id].to_i if @fields.has_key?(:user_id)
@fields[:category_id] = @fields[:category_id].to_i if @fields.has_key?(:category_id)
# always reset edit_reason unless provided
@fields[:edit_reason] = nil unless @fields.has_key?(:edit_reason)
return false unless should_revise?
@post.acting_user = @editor
@topic.acting_user = @editor
@revised_at = @opts[:revised_at] || Time.now
@last_version_at = @post.last_version_at || Time.now
@version_changed = false
@post_successfully_saved = true
@topic_successfully_saved = true
Post.transaction do
revise_post
# TODO these callbacks are being called in a transaction
# it is kind of odd, cause the callback is called before_edit
# but the post is already edited at this point
# trouble is that much of the logic of should I edit? is deeper
# down so yanking this in front of the transaction will lead to
# false positives. This system needs a review
# TODO: these callbacks are being called in a transaction
# it is kind of odd, because the callback is called "before_edit"
# but the post is already edited at this point
# Trouble is that much of the logic of should I edit? is deeper
# down so yanking this in front of the transaction will lead to
# false positive.
plugin_callbacks
update_category_description
update_topic_excerpt
@post.advance_draft_sequence
revise_topic
advance_draft_sequence
end
# WARNING: do not pull this into the transaction, it can fire events in
# sidekiq before the post is done saving leading to corrupt state
# WARNING: do not pull this into the transaction
# it can fire events in sidekiq before the post is done saving
# leading to corrupt state
post_process_post
update_topic_word_counts
alert_users
publish_changes
grant_badge
PostAlerter.new.after_save_post(@post)
@post.publish_change_to_clients! :revised
BadgeGranter.queue_badge_grant(Badge::Trigger::PostRevision, post: @post)
true
@post_successfully_saved && @topic_successfully_saved
end
private
def cleanup_whitespaces(raw)
TextCleaner.normalize_whitespaces(raw).gsub(/\s+\z/, "")
end
def should_revise?
@post.raw != @new_raw || @opts[:changed_owner]
post_changed? || topic_changed?
end
def post_changed?
POST_TRACKED_FIELDS.each do |field|
return true if @fields.has_key?(field) && @fields[field] != @post.send(field)
end
false
end
def topic_changed?
TOPIC_TRACKED_FIELDS.each do |field|
return true if @fields.has_key?(field) && @fields[field] != @topic.send(field)
end
false
end
def revise_post
if should_create_new_version?
revise_and_create_new_version
else
update_post
revise
end
end
def plugin_callbacks
DiscourseEvent.trigger :before_edit_post, @post
DiscourseEvent.trigger :validate_post, @post
end
def get_revised_at
@opts[:revised_at] || Time.now
end
def should_create_new_version?
@post.last_editor_id != @editor.id ||
get_revised_at - @post.last_version_at > SiteSetting.ninja_edit_window.to_i ||
@opts[:changed_owner] == true ||
edited_by_another_user? || !ninja_edit? || owner_changed? || force_new_version?
end
def edited_by_another_user?
@post.last_editor_id != @editor.id
end
def ninja_edit?
@revised_at - @last_version_at <= SiteSetting.ninja_edit_window.to_i
end
def owner_changed?
@fields.has_key?(:user_id) && @fields[:user_id] != @post.user_id
end
def force_new_version?
@opts[:force_new_version] == true
end
def revise_and_create_new_version
@version_changed = true
@post.version += 1
@post.last_version_at = get_revised_at
@post.public_version += 1
@post.last_version_at = @revised_at
revise
perform_edit
bump_topic
end
def revise
update_post
EditRateLimiter.new(@editor).performed! unless @opts[:bypass_rate_limiter] == true
bump_topic unless @opts[:bypass_bump]
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)
TopicTrackingState.publish_latest(@post.topic)
end
end
def update_topic_word_counts
Topic.exec_sql("UPDATE topics SET word_count = (SELECT SUM(COALESCE(posts.word_count, 0))
FROM posts WHERE posts.topic_id = :topic_id)
WHERE topics.id = :topic_id", topic_id: @post.topic_id)
update_topic if topic_changed?
create_or_update_revision
end
def update_post
@post.raw = @new_raw
@post.word_count = @new_raw.scan(/\w+/).size
@post.last_editor_id = @editor.id
@post.edit_reason = @opts[:edit_reason] if @opts[:edit_reason]
@post.user_id = @opts[:new_user].id if @opts[:new_user]
@post.self_edits += 1 if @editor == @post.user
if @editor == @post.user && @post.hidden && @post.hidden_reason_id == Post.hidden_reasons[:flag_threshold_reached]
PostAction.clear_flags!(@post, Discourse.system_user)
@post.unhide!
POST_TRACKED_FIELDS.each do |field|
@post.send("#{field}=", @fields[field]) if @fields.has_key?(field)
end
@post.extract_quoted_post_numbers
@post.save(validate: !@opts[:skip_validations])
@post.last_editor_id = @editor.id
@post.word_count = @fields[:raw].scan(/\w+/).size if @fields.has_key?(:raw)
@post.self_edits += 1 if self_edit?
clear_flags_and_unhide_post
@post.extract_quoted_post_numbers
@post_successfully_saved = @post.save(validate: !@opts[:skip_validations])
@post.save_reply_relationships
end
def update_category_description
# If we're revising the first post, we might have to update the category description
def self_edit?
@editor == @post.user
end
def clear_flags_and_unhide_post
return unless editing_a_flagged_and_hidden_post?
PostAction.clear_flags!(@post, Discourse.system_user)
@post.unhide!
end
def editing_a_flagged_and_hidden_post?
self_edit? &&
@post.hidden &&
@post.hidden_reason_id == Post.hidden_reasons[:flag_threshold_reached]
end
def update_topic
@topic.title = @fields[:title] if @fields.has_key?(:title)
Topic.transaction do
@topic_successfully_saved = @topic.change_category_to_id(@fields[:category_id]) if @fields.has_key?(:category_id)
@topic_successfully_saved &&= @topic.save(validate: !@opts[:skip_validations])
end
end
def create_or_update_revision
if @version_changed
create_revision
else
update_revision
end
end
def create_revision
modifications = post_changes.merge(topic_changes)
PostRevision.create!(
user_id: @post.last_editor_id,
post_id: @post.id,
number: @post.version,
modifications: modifications
)
end
def update_revision
return unless revision = PostRevision.find_by(post_id: @post.id, number: @post.version)
revision.user_id = @post.last_editor_id
modifications = post_changes.merge(topic_changes)
modifications.keys.each do |field|
if revision.modifications.has_key?(field)
old_value = revision.modifications[field][0]
new_value = modifications[field][1]
revision.modifications[field] = [old_value, new_value]
else
revision.modifications[field] = modifications[field]
end
end
revision.save
end
def post_changes
@post.previous_changes.slice(*POST_TRACKED_FIELDS)
end
def topic_changes
@topic.previous_changes.slice(*TOPIC_TRACKED_FIELDS)
end
def perform_edit
return if bypass_rate_limiter?
EditRateLimiter.new(@editor).performed!
end
def bypass_rate_limiter?
@opts[:bypass_rate_limiter] == true
end
def bump_topic
return if bypass_bump? || !is_last_post?
@topic.update_column(:bumped_at, Time.now)
TopicTrackingState.publish_latest(@topic)
end
def bypass_bump?
@opts[:bypass_bump] == true
end
def is_last_post?
!Post.where(topic_id: @topic.id)
.where("post_number > ?", @post.post_number)
.exists?
end
def plugin_callbacks
DiscourseEvent.trigger(:before_edit_post, @post)
DiscourseEvent.trigger(:validate_post, @post)
end
def revise_topic
return unless @post.post_number == 1
# Is there a category with our topic id?
category = Category.find_by(topic_id: @post.topic_id)
return unless category.present?
update_topic_excerpt
update_category_description
end
def update_topic_excerpt
excerpt = @post.excerpt(220, strip_links: true)
@topic.update_column(:excerpt, excerpt)
end
def update_category_description
return unless category = Category.find_by(topic_id: @topic.id)
# If found, update its description
body = @post.cooked
matches = body.scan(/\<p\>(.*)\<\/p\>/)
if matches && matches[0] && matches[0][0]
@ -143,12 +274,35 @@ class PostRevisor
end
end
def update_topic_excerpt
@post.topic.update_column(:excerpt, @post.excerpt(220, strip_links: true)) if @post.post_number == 1
def advance_draft_sequence
@post.advance_draft_sequence
end
def post_process_post
@post.invalidate_oneboxes = true
@post.trigger_post_process
end
def update_topic_word_counts
Topic.exec_sql("UPDATE topics
SET word_count = (
SELECT SUM(COALESCE(posts.word_count, 0))
FROM posts
WHERE posts.topic_id = :topic_id
)
WHERE topics.id = :topic_id", topic_id: @topic.id)
end
def alert_users
PostAlerter.new.after_save_post(@post)
end
def publish_changes
@post.publish_change_to_clients!(:revised)
end
def grant_badge
BadgeGranter.queue_badge_grant(Badge::Trigger::PostRevision, post: @post)
end
end

2
lib/tasks/posts.rake
View File

@ -48,7 +48,7 @@ task 'posts:normalize_code' => :environment do
Post.where("raw like '%<pre>%<code>%'").each do |p|
normalized = Import::Normalize.normalize_code_blocks(p.raw, lang)
if normalized != p.raw
p.revise(Discourse.system_user, normalized)
p.revise(Discourse.system_user, { raw: normalized })
putc "."
i += 1
end

2
script/import_scripts/phpbb3.rb
View File

@ -413,7 +413,7 @@ class ImportScripts::PhpBB3 < ImportScripts::Base
end
if new_raw != post.raw
PostRevisor.new(post).revise!(post.user, new_raw, {bypass_bump: true, edit_reason: 'Migrate from PHPBB3'})
PostRevisor.new(post).revise!(post.user, { raw: new_raw }, { bypass_bump: true, edit_reason: 'Migrate from PHPBB3' })
end
end

2
spec/components/category_list_spec.rb
View File

@ -86,7 +86,7 @@ describe CategoryList do
end
context "with a topic in a category" do
let!(:topic) { Fabricate(:topic, category: topic_category)}
let!(:topic) { Fabricate(:topic, category: topic_category) }
let(:category) { category_list.categories.first }
it "should return the category" do

49
spec/components/post_revisor_spec.rb
View File

@ -16,7 +16,7 @@ describe PostRevisor do
describe 'with the same body' do
it "doesn't change version" do
lambda {
subject.revise!(post.user, post.raw).should == false
subject.revise!(post.user, { raw: post.raw }).should == false
post.reload
}.should_not change(post, :version)
end
@ -25,10 +25,11 @@ describe PostRevisor do
describe 'ninja editing' do
it 'correctly applies edits' do
SiteSetting.ninja_edit_window = 1.minute.to_i
subject.revise!(post.user, 'updated body', revised_at: post.updated_at + 10.seconds)
subject.revise!(post.user, { raw: 'updated body' }, revised_at: post.updated_at + 10.seconds)
post.reload
post.version.should == 1
post.public_version.should == 1
post.revisions.size.should == 0
post.last_version_at.should == first_version_at
subject.category_changed.should be_blank
@ -41,7 +42,7 @@ describe PostRevisor do
before do
SiteSetting.stubs(:ninja_edit_window).returns(1.minute.to_i)
subject.revise!(post.user, 'updated body', revised_at: revised_at)
subject.revise!(post.user, { raw: 'updated body' }, revised_at: revised_at)
post.reload
end
@ -49,11 +50,12 @@ describe PostRevisor do
subject.category_changed.should be_blank
end
it 'updates the version' do
it 'updates the versions' do
post.version.should == 2
post.public_version.should == 2
end
it 'creates a new version' do
it 'creates a new revision' do
post.revisions.size.should == 1
end
@ -64,12 +66,13 @@ describe PostRevisor do
describe "new edit window" do
before do
subject.revise!(post.user, 'yet another updated body', revised_at: revised_at)
subject.revise!(post.user, { raw: 'yet another updated body' }, revised_at: revised_at)
post.reload
end
it "doesn't create a new version if you do another" do
post.version.should == 2
post.public_version.should == 2
end
it "doesn't change last_version_at" do
@ -85,12 +88,13 @@ describe PostRevisor do
let!(:new_revised_at) {revised_at + 2.minutes}
before do
subject.revise!(post.user, 'yet another, another updated body', revised_at: new_revised_at)
subject.revise!(post.user, { raw: 'yet another, another updated body' }, revised_at: new_revised_at)
post.reload
end
it "does create a new version after the edit window" do
post.version.should == 3
post.public_version.should == 3
end
it "does create a new version after the edit window" do
@ -116,7 +120,7 @@ describe PostRevisor do
context "one paragraph description" do
before do
subject.revise!(post.user, new_description)
subject.revise!(post.user, { raw: new_description })
category.reload
end
@ -131,7 +135,7 @@ describe PostRevisor do
context "multiple paragraph description" do
before do
subject.revise!(post.user, "#{new_description}\n\nOther content goes here.")
subject.revise!(post.user, { raw: "#{new_description}\n\nOther content goes here." })
category.reload
end
@ -147,7 +151,7 @@ describe PostRevisor do
context 'when updating back to the original paragraph' do
before do
category.update_column(:description, 'this is my description')
subject.revise!(post.user, Category.post_template)
subject.revise!(post.user, { raw: Category.post_template })
category.reload
end
@ -167,7 +171,7 @@ describe PostRevisor do
it "triggers a rate limiter" do
EditRateLimiter.any_instance.expects(:performed!)
subject.revise!(changed_by, 'updated body')
subject.revise!(changed_by, { raw: 'updated body' })
end
end
@ -178,7 +182,7 @@ describe PostRevisor do
SiteSetting.stubs(:newuser_max_images).returns(0)
url = "http://i.imgur.com/wfn7rgU.jpg"
Oneboxer.stubs(:onebox).with(url, anything).returns("<img src='#{url}'>")
subject.revise!(changed_by, "So, post them here!\n#{url}")
subject.revise!(changed_by, { raw: "So, post them here!\n#{url}" })
end
it "allows an admin to insert images into a new user's post" do
@ -196,7 +200,7 @@ describe PostRevisor do
SiteSetting.stubs(:newuser_max_images).returns(0)
url = "http://i.imgur.com/FGg7Vzu.gif"
Oneboxer.stubs(:cached_onebox).with(url, anything).returns("<img src='#{url}'>")
subject.revise!(post.user, "So, post them here!\n#{url}")
subject.revise!(post.user, { raw: "So, post them here!\n#{url}" })
end
it "doesn't allow images to be inserted" do
@ -208,7 +212,7 @@ describe PostRevisor do
describe 'with a new body' do
let(:changed_by) { Fabricate(:coding_horror) }
let!(:result) { subject.revise!(changed_by, "lets update the body") }
let!(:result) { subject.revise!(changed_by, { raw: "lets update the body" }) }
it 'returns true' do
result.should == true
@ -222,8 +226,9 @@ describe PostRevisor do
post.invalidate_oneboxes.should == true
end
it 'increased the version' do
it 'increased the versions' do
post.version.should == 2
post.public_version.should == 2
end
it 'has the new revision' do
@ -242,16 +247,14 @@ describe PostRevisor do
context 'second poster posts again quickly' do
before do
SiteSetting.expects(:ninja_edit_window).returns(1.minute.to_i)
subject.revise!(changed_by, 'yet another updated body', revised_at: post.updated_at + 10.seconds)
SiteSetting.stubs(:ninja_edit_window).returns(1.minute.to_i)
subject.revise!(changed_by, { raw: 'yet another updated body' }, revised_at: post.updated_at + 10.seconds)
post.reload
end
it 'is a ninja edit, because the second poster posted again quickly' do
post.version.should == 2
end
it 'is a ninja edit, because the second poster posted again quickly' do
post.public_version.should == 2
post.revisions.size.should == 1
end
end
@ -262,19 +265,19 @@ describe PostRevisor do
revisor = described_class.new(post)
first_post = topic.posts.by_post_number.first
expect {
revisor.revise!(first_post.user, 'Edit the first post', revised_at: first_post.updated_at + 10.seconds)
revisor.revise!(first_post.user, { raw: 'Edit the first post' }, revised_at: first_post.updated_at + 10.seconds)
topic.reload
}.to change { topic.excerpt }
second_post = Fabricate(:post, post_args.merge(post_number: 2, topic_id: topic.id))
expect {
described_class.new(second_post).revise!(second_post.user, 'Edit the 2nd post')
described_class.new(second_post).revise!(second_post.user, { raw: 'Edit the 2nd post' })
topic.reload
}.to_not change { topic.excerpt }
end
end
it "doesn't strip starting whitespaces" do
subject.revise!(post.user, " <-- whitespaces --> ")
subject.revise!(post.user, { raw: " <-- whitespaces --> " })
post.reload
post.raw.should == " <-- whitespaces -->"
end

11
spec/controllers/posts_controller_spec.rb
View File

@ -307,7 +307,7 @@ describe PostsController do
end
it "calls revise with valid parameters" do
PostRevisor.any_instance.expects(:revise!).with(post.user, 'edited body', edit_reason: 'typo')
PostRevisor.any_instance.expects(:revise!).with(post.user, { raw: 'edited body' , edit_reason: 'typo' })
xhr :put, :update, update_params
end
@ -605,7 +605,8 @@ describe PostsController do
describe "revisions" do
let(:post_revision) { Fabricate(:post_revision) }
let(:post) { Fabricate(:post, version: 2) }
let(:post_revision) { Fabricate(:post_revision, post: post) }
it "throws an exception when revision is < 2" do
expect {
@ -636,7 +637,7 @@ describe PostsController do
it "ensures poster can see the revisions" do
user = log_in(:active_user)
post = Fabricate(:post, user: user)
post = Fabricate(:post, user: user, version: 3)
pr = Fabricate(:post_revision, user: user, post: post)
xhr :get, :revisions, post_id: pr.post_id, revision: pr.number
response.should be_success
@ -663,7 +664,7 @@ describe PostsController do
context "deleted post" do
let(:admin) { log_in(:admin) }
let(:deleted_post) { Fabricate(:post, user: admin) }
let(:deleted_post) { Fabricate(:post, user: admin, version: 3) }
let(:deleted_post_revision) { Fabricate(:post_revision, user: admin, post: deleted_post) }
before { deleted_post.trash!(admin) }
@ -677,7 +678,7 @@ describe PostsController do
context "deleted topic" do
let(:admin) { log_in(:admin) }
let(:deleted_topic) { Fabricate(:topic, user: admin) }
let(:post) { Fabricate(:post, user: admin, topic: deleted_topic) }
let(:post) { Fabricate(:post, user: admin, topic: deleted_topic, version: 3) }
let(:post_revision) { Fabricate(:post_revision, user: admin, post: post) }
before { deleted_topic.trash!(admin) }

3
spec/controllers/topics_controller_spec.rb
View File

@ -733,6 +733,7 @@ describe TopicsController do
describe 'when logged in' do
before do
@topic = Fabricate(:topic, user: log_in)
Fabricate(:post, topic: @topic)
end
describe 'without permission' do
@ -778,7 +779,7 @@ describe TopicsController do
it "returns errors with invalid categories" do
Topic.any_instance.expects(:change_category_to_id).returns(false)
xhr :put, :update, topic_id: @topic.id, slug: @topic.title
xhr :put, :update, topic_id: @topic.id, slug: @topic.title, category_id: -1
expect(response).not_to be_success
end

2
spec/fabricators/post_revision_fabricator.rb
View File

@ -1,7 +1,7 @@
Fabricator(:post_revision) do
post
user
number 3
number 2
modifications do
{ "cooked" => ["<p>BEFORE</p>", "<p>AFTER</p>"], "raw" => ["BEFORE", "AFTER"] }
end

2
spec/models/category_spec.rb
View File

@ -368,7 +368,7 @@ describe Category do
post = create_post(user: @category.user, category: @category.name)
SiteSetting.stubs(:ninja_edit_window).returns(1.minute.to_i)
post.revise(post.user, 'updated body', revised_at: post.updated_at + 2.minutes)
post.revise(post.user, { raw: 'updated body' }, revised_at: post.updated_at + 2.minutes)
Category.update_stats
@category.reload

2
spec/models/draft_spec.rb
View File

@ -73,7 +73,7 @@ describe Draft do
it 'nukes the post draft when a post is revised' do
p = Fabricate(:post)
Draft.set(p.user, p.topic.draft_key, 0,'hello')
p.revise(p.user, 'another test')
p.revise(p.user, { raw: 'another test' })
s = DraftSequence.current(p.user, p.topic.draft_key)
Draft.get(p.user, p.topic.draft_key, s).should == nil
end

4
spec/models/post_action_spec.rb
View File

@ -297,7 +297,7 @@ describe PostAction do
post.hidden_reason_id.should == Post.hidden_reasons[:flag_threshold_reached]
post.topic.visible.should == false
post.revise(post.user, post.raw + " ha I edited it ")
post.revise(post.user, { raw: post.raw + " ha I edited it " })
post.reload
@ -316,7 +316,7 @@ describe PostAction do
post.hidden_reason_id.should == Post.hidden_reasons[:flag_threshold_reached_again]
post.topic.visible.should == false
post.revise(post.user, post.raw + " ha I edited it again ")
post.revise(post.user, { raw: post.raw + " ha I edited it again " })
post.reload

6
spec/models/post_alert_observer_spec.rb
View File

@ -36,7 +36,7 @@ describe PostAlertObserver do
context 'when editing a post' do
it 'notifies a user of the revision' do
lambda {
post.revise(evil_trout, "world is the new body of the message")
post.revise(evil_trout, { raw: "world is the new body of the message" })
}.should change(post.user.notifications, :count).by(1)
end
@ -47,13 +47,13 @@ describe PostAlertObserver do
it 'notifies a user of the revision made by another user' do
lambda {
post.revise(evil_trout, "world is the new body of the message")
post.revise(evil_trout, { raw: "world is the new body of the message" })
}.should change(post.user.notifications, :count).by(1)
end
it 'does not notifiy a user of the revision made by the system user' do
lambda {
post.revise(Discourse.system_user, "world is the new body of the message")
post.revise(Discourse.system_user, { raw: "world is the new body of the message" })
}.should_not change(post.user.notifications, :count)
end

119
spec/models/post_revision_spec.rb
View File

@ -1,119 +0,0 @@
require 'spec_helper'
require_dependency 'post_revision'
describe PostRevision do
before do
@number = 1
end
def create_rev(modifications, post_id=1)
@number += 1
PostRevision.create!(post_id: post_id, user_id: 1, number: @number, modifications: modifications)
end
it "ignores deprecated current values in history" do
p = PostRevision.new(modifications: {"foo" => ["bar", "bar1"]})
p.previous("foo").should == "bar"
p.current("foo").should == "bar"
end
it "can fallback to previous revisions if needed" do
r1 = create_rev("foo" => ["A", "B"])
r2 = create_rev("foo" => ["C", "D"])
r1.current("foo").should == "C"
r2.current("foo").should == "C"
r2.previous("foo").should == "C"
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 post for current rev only if needed" do
post = Fabricate(:post)
r = create_rev({"raw" => ["A"], "cooked" => ["AA"]}, post.id)
r.current("raw").should == post.raw
r.previous("raw").should == "A"
r.current("cooked").should == post.cooked
r.previous("cooked").should == "AA"
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
r1 = create_rev({"title" => ["hello"]})
r2 = create_rev({"title" => ["frog"]})
r1.title_changes[:inline].should =~ /frog.*hello/
r1.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")
r1 = create_rev({"category_id" => [cat1.id, cat2.id]})
r2 = create_rev({"category_id" => [cat2.id, cat1.id]})
changes = r1.category_changes
changes[:previous_category_id].should == cat1.id
changes[:current_category_id].should == cat2.id
end
it "can find wiki changes" do
r1 = create_rev("wiki" => [false])
r2 = create_rev("wiki" => [true])
changes = r1.wiki_changes
changes[:previous_wiki].should == false
changes[:current_wiki].should == true
end
it "can find post_type changes" do
r1 = create_rev("post_type" => [1])
r2 = create_rev("post_type" => [2])
changes = r1.post_type_changes
changes[:previous_post_type].should == 1
changes[:current_post_type].should == 2
end
it "hides revisions that were hidden" do
r1 = create_rev({"raw" => ["one"]})
r2 = create_rev({"raw" => ["two"]})
r3 = create_rev({"raw" => ["three"]})
r2.hide!
r1.current("raw").should == "three"
r2.previous("raw").should == "one"
end
it "shows revisions that were shown" do
r1 = create_rev({"raw" => ["one"]})
r2 = create_rev({"raw" => ["two"]})
r3 = create_rev({"raw" => ["three"]})
r2.hide!
r2.show!
r2.previous("raw").should == "two"
r1.current("raw").should == "two"
end
end

109
spec/models/post_spec.rb
View File

@ -161,7 +161,7 @@ describe Post do
post_no_images.user.trust_level = TrustLevel[0]
post_no_images.save
-> {
post_no_images.revise(post_no_images.user, post_two_images.raw)
post_no_images.revise(post_no_images.user, { raw: post_two_images.raw })
post_no_images.reload
}.should_not change(post_no_images, :raw)
end
@ -209,7 +209,7 @@ describe Post do
post_no_attachments.user.trust_level = TrustLevel[0]
post_no_attachments.save
-> {
post_no_attachments.revise(post_no_attachments.user, post_two_attachments.raw)
post_no_attachments.revise(post_no_attachments.user, { raw: post_two_attachments.raw })
post_no_attachments.reload
}.should_not change(post_no_attachments, :raw)
end
@ -468,83 +468,56 @@ describe Post do
it 'has no revision' do
post.revisions.size.should == 0
first_version_at.should be_present
post.revise(post.user, post.raw).should == false
post.revise(post.user, { raw: post.raw }).should == false
end
describe 'with the same body' do
it "doesn't change version" do
lambda { post.revise(post.user, post.raw); post.reload }.should_not change(post, :version)
lambda { post.revise(post.user, { raw: post.raw }); post.reload }.should_not change(post, :version)
end
end
describe 'ninja editing' do
before do
SiteSetting.expects(:ninja_edit_window).returns(1.minute.to_i)
post.revise(post.user, 'updated body', revised_at: post.updated_at + 10.seconds)
post.reload
end
describe 'ninja editing & edit windows' do
it 'causes no update' do
before { SiteSetting.stubs(:ninja_edit_window).returns(1.minute.to_i) }
it 'works' do
revised_at = post.updated_at + 2.minutes
new_revised_at = revised_at + 2.minutes
# ninja edit
post.revise(post.user, { raw: 'updated body' }, revised_at: post.updated_at + 10.seconds)
post.reload
post.version.should == 1
post.public_version.should == 1
post.revisions.size.should == 0
post.last_version_at.should == first_version_at
end
post.last_version_at.to_i.should == first_version_at.to_i
end
describe 'revision much later' do
let!(:revised_at) { post.updated_at + 2.minutes }
before do
SiteSetting.stubs(:ninja_edit_window).returns(1.minute.to_i)
post.revise(post.user, 'updated body', revised_at: revised_at)
# revision much later
post.revise(post.user, { raw: 'another updated body' }, revised_at: revised_at)
post.reload
end
it 'updates the version' do
post.version.should == 2
post.public_version.should == 2
post.revisions.size.should == 1
post.last_version_at.to_i.should == revised_at.to_i
end
describe "new edit window" do
before do
post.revise(post.user, 'yet another updated body', revised_at: revised_at)
post.reload
end
it "doesn't create a new version if you do another" do
post.version.should == 2
end
it "doesn't change last_version_at" do
post.last_version_at.to_i.should == revised_at.to_i
end
context "after second window" do
let!(:new_revised_at) {revised_at + 2.minutes}
before do
post.revise(post.user, 'yet another, another updated body', revised_at: new_revised_at)
post.reload
end
it "does create a new version after the edit window" do
post.version.should == 3
end
it "does create a new version after the edit window" do
post.last_version_at.to_i.should == new_revised_at.to_i
end
end
# new edit window
post.revise(post.user, { raw: 'yet another updated body' }, revised_at: revised_at + 10.seconds)
post.reload
post.version.should == 2
post.public_version.should == 2
post.revisions.size.should == 1
post.last_version_at.to_i.should == revised_at.to_i
# after second window
post.revise(post.user, { raw: 'yet another, another updated body' }, revised_at: new_revised_at)
post.reload
post.version.should == 3
post.public_version.should == 3
post.revisions.size.should == 2
post.last_version_at.to_i.should == new_revised_at.to_i
end
end
@ -554,38 +527,40 @@ describe Post do
it "triggers a rate limiter" do
EditRateLimiter.any_instance.expects(:performed!)
post.revise(changed_by, 'updated body')
post.revise(changed_by, { raw: 'updated body' })
end
end
describe 'with a new body' do
let(:changed_by) { Fabricate(:coding_horror) }
let!(:result) { post.revise(changed_by, 'updated body') }
let!(:result) { post.revise(changed_by, { raw: 'updated body' }) }
it 'acts correctly' do
result.should == true
post.raw.should == 'updated body'
post.invalidate_oneboxes.should == true
post.version.should == 2
post.public_version.should == 2
post.revisions.size.should == 1
post.revisions.first.user.should be_present
end
context 'second poster posts again quickly' do
before do
SiteSetting.expects(:ninja_edit_window).returns(1.minute.to_i)
post.revise(changed_by, 'yet another updated body', revised_at: post.updated_at + 10.seconds)
post.reload
end
it 'is a ninja edit, because the second poster posted again quickly' do
SiteSetting.expects(:ninja_edit_window).returns(1.minute.to_i)
post.revise(changed_by, { raw: 'yet another updated body' }, revised_at: post.updated_at + 10.seconds)
post.reload
post.version.should == 2
post.public_version.should == 2
post.revisions.size.should == 1
end
end
end
end

2
spec/models/topic_link_spec.rb
View File

@ -118,7 +118,7 @@ http://b.com/#{'a'*500}
context 'removing a link' do
before do
post.revise(post.user, "no more linkies")
post.revise(post.user, { raw: "no more linkies" })
TopicLink.extract_from(post)
end

79
spec/models/topic_spec.rb
View File

@ -352,21 +352,21 @@ describe Topic do
it "doesn't bump the topic on an edit to the last post that doesn't result in a new version" do
lambda {
SiteSetting.expects(:ninja_edit_window).returns(5.minutes)
@last_post.revise(@last_post.user, 'updated contents', revised_at: @last_post.created_at + 10.seconds)
@last_post.revise(@last_post.user, { raw: 'updated contents' }, revised_at: @last_post.created_at + 10.seconds)
@topic.reload
}.should_not change(@topic, :bumped_at)
end
it "bumps the topic when a new version is made of the last post" do
lambda {
@last_post.revise(Fabricate(:moderator), 'updated contents')
@last_post.revise(Fabricate(:moderator), { raw: 'updated contents' })
@topic.reload
}.should change(@topic, :bumped_at)
end
it "doesn't bump the topic when a post that isn't the last post receives a new version" do
lambda {
@earlier_post.revise(Fabricate(:moderator), 'updated contents')
@earlier_post.revise(Fabricate(:moderator), { raw: 'updated contents' })
@topic.reload
}.should_not change(@topic, :bumped_at)
end
@ -689,16 +689,17 @@ describe Topic do
end
describe 'with category' do
before do
@category = Fabricate(:category)
end
it "should not increase the topic_count with no category" do
lambda { Fabricate(:topic, user: @category.user); @category.reload }.should_not change(@category, :topic_count)
-> { Fabricate(:topic, user: @category.user); @category.reload }.should_not change(@category, :topic_count)
end
it "should increase the category's topic_count" do
lambda { Fabricate(:topic, user: @category.user, category_id: @category.id); @category.reload }.should change(@category, :topic_count).by(1)
-> { Fabricate(:topic, user: @category.user, category_id: @category.id); @category.reload }.should change(@category, :topic_count).by(1)
end
end
@ -777,74 +778,6 @@ describe Topic do
end
end
describe 'revisions' do
let(:post) { Fabricate(:post) }
let(:topic) { post.topic }
it "has no revisions by default" do
post.revisions.size.should == 0
end
context 'changing title' do
before do
topic.title = "new title for the topic"
topic.save
end
it "creates a new revision" do
post.revisions.size.should == 1
end
end
context 'changing category' do
let(:category) { Fabricate(:category) }
it "creates a new revision" do
topic.change_category_to_id(category.id)
post.revisions.size.should == 1
end
it "does nothing for private messages" do
topic.archetype = "private_message"
topic.category_id = nil
topic.change_category_to_id(category.id)
topic.category_id.should == nil
end
context "removing a category" do
before do
topic.change_category_to_id(category.id)
topic.change_category_to_id(nil)
end
it "creates a new revision" do
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
end
context 'bumping the topic' do
before do
topic.bumped_at = 10.minutes.from_now
topic.save
end
it "doesn't create a new version" do
post.revisions.size.should == 0
end
end
end
describe 'change_category' do
before do

9
spec/models/user_action_spec.rb
View File

@ -49,7 +49,6 @@ describe UserAction do
end
it 'includes the events correctly' do
mystats = stats_for_user(user)
expecting = [UserAction::NEW_TOPIC, UserAction::NEW_PRIVATE_MESSAGE, UserAction::GOT_PRIVATE_MESSAGE, UserAction::BOOKMARK].sort
mystats.should == expecting
@ -66,7 +65,6 @@ describe UserAction do
stream_count.should == 0
# groups
category = Fabricate(:category, read_restricted: true)
public_topic.recover!
@ -90,18 +88,16 @@ describe UserAction do
# duplicate should not exception out
log_test_action
# recategorize belongs to the right user
category2 = Fabricate(:category)
admin = Fabricate(:admin)
public_topic.acting_user = admin
public_topic.change_category_to_id(category2.id)
public_post.revise(admin, { category_id: category2.id})
action = UserAction.stream(user_id: public_topic.user_id, guardian: Guardian.new)[0]
action.acting_user_id.should == admin.id
action.action_type.should == UserAction::EDIT
end
end
describe 'when user likes' do
@ -121,7 +117,6 @@ describe UserAction do
it "creates a new stream entry" do
PostAction.act(liker, post, PostActionType.types[:like])
likee_stream.count.should == @old_count + 1
end
context "successful like" do

2
spec/services/badge_granter_spec.rb
View File

@ -192,7 +192,7 @@ describe BadgeGranter do
UserBadge.where(user_id: user.id, badge_id: Badge::Editor).count.should eq(0)
PostRevisor.new(post).revise!(user, "This is my new test 1235 123")
PostRevisor.new(post).revise!(user, { raw: "This is my new test 1235 123" })
BadgeGranter.process_queue!
UserBadge.where(user_id: user.id, badge_id: Badge::Editor).count.should eq(1)

4
spec/services/post_alerter_spec.rb
View File

@ -20,7 +20,7 @@ describe PostAlerter do
it "won't notify the user a second time on revision" do
p1 = create_post_with_alerts(raw: '[quote="Evil Trout, post:1"]whatup[/quote]')
lambda {
p1.revise(p1.user, '[quote="Evil Trout, post:1"]whatup now?[/quote]')
p1.revise(p1.user, { raw: '[quote="Evil Trout, post:1"]whatup now?[/quote]' })
}.should_not change(evil_trout.notifications, :count)
end
@ -67,7 +67,7 @@ describe PostAlerter do
it "won't notify the user a second time on revision" do
mention_post
lambda {
mention_post.revise(mention_post.user, "New raw content that still mentions @eviltrout")
mention_post.revise(mention_post.user, { raw: "New raw content that still mentions @eviltrout" })
}.should_not change(evil_trout.notifications, :count)
end