* Reworked how post versioning works, now more closely resembles the 1.18 strategy

This commit is contained in:
albert
2011-01-26 18:10:49 -05:00
parent 683d4583ac
commit f7e2344b9f
21 changed files with 292 additions and 308 deletions

View File

@@ -1,6 +0,0 @@
class PostHistoriesController < ApplicationController
def index
@search = PostHistory.search(params[:search])
@histories = @search.paginate(:page => params[:page], :per_page => 20, :order => "updated_at DESC")
end
end

View File

@@ -0,0 +1,6 @@
class PostVersionsController < ApplicationController
def index
@search = PostVersion.search(params[:search])
@post_versions = @search.paginate(:page => params[:page], :per_page => 20, :order => "updated_at DESC")
end
end

View File

@@ -5,7 +5,4 @@ class PostVotesController < ApplicationController
rescue PostVote::Error => x rescue PostVote::Error => x
@error = x @error = x
end end
def destroy
end
end end

View File

@@ -4,6 +4,11 @@ class JanitorTrial < ActiveRecord::Base
after_create :promote_user after_create :promote_user
after_destroy :create_feedback after_destroy :create_feedback
validates_presence_of :user validates_presence_of :user
before_validation :initialize_creator
def initialize_creator
self.creator_id = CurrentUser.id
end
def send_dmail def send_dmail
body = "You have been selected as a test janitor. You can now approve pending posts and have access to the moderation interface.\n\nOver the next several weeks your approvals will be monitored. If the majority of them are quality uploads, then you will be promoted to full janitor status which grants you the ability to delete and undelete posts, ban users, and revert tag changes from vandals. If you fail the trial period, you will be demoted back to your original level and you'll receive a negative user record indicating you previously attempted and failed a test janitor trial.\n\nThere is a minimum quota of 5 approvals a week to indicate that you are being active. Remember, the goal isn't to approve as much as possible. It's to filter out borderline-quality art.\n\nIf you have any questions please respond to this message." body = "You have been selected as a test janitor. You can now approve pending posts and have access to the moderation interface.\n\nOver the next several weeks your approvals will be monitored. If the majority of them are quality uploads, then you will be promoted to full janitor status which grants you the ability to delete and undelete posts, ban users, and revert tag changes from vandals. If you fail the trial period, you will be demoted back to your original level and you'll receive a negative user record indicating you previously attempted and failed a test janitor trial.\n\nThere is a minimum quota of 5 approvals a week to indicate that you are being active. Remember, the goal isn't to approve as much as possible. It's to filter out borderline-quality art.\n\nIf you have any questions please respond to this message."

View File

@@ -3,7 +3,7 @@ class Post < ActiveRecord::Base
attr_accessor :old_tag_string, :old_parent_id attr_accessor :old_tag_string, :old_parent_id
after_destroy :delete_files after_destroy :delete_files
after_save :update_history after_save :create_version
after_save :update_parent_on_save after_save :update_parent_on_save
before_save :merge_old_tags before_save :merge_old_tags
before_save :normalize_tags before_save :normalize_tags
@@ -16,8 +16,7 @@ class Post < ActiveRecord::Base
belongs_to :parent, :class_name => "Post" belongs_to :parent, :class_name => "Post"
has_one :unapproval, :dependent => :destroy has_one :unapproval, :dependent => :destroy
has_one :upload, :dependent => :destroy has_one :upload, :dependent => :destroy
has_one :moderation_detail, :class_name => "PostModerationDetail", :dependent => :destroy has_many :versions, :class_name => "PostVersion", :dependent => :destroy
has_one :history, :class_name => "PostHistory"
has_many :votes, :class_name => "PostVote", :dependent => :destroy has_many :votes, :class_name => "PostVote", :dependent => :destroy
has_many :notes, :dependent => :destroy has_many :notes, :dependent => :destroy
has_many :comments has_many :comments
@@ -240,24 +239,6 @@ class Post < ActiveRecord::Base
end end
end end
module HistoryMethods
def revisions
if history.nil?
update_history
end
history.revisions
end
def update_history
if history.nil?
create_history
end
history << self
end
end
module TagMethods module TagMethods
def tag_array def tag_array
@tag_array ||= Tag.scan_tags(tag_string) @tag_array ||= Tag.scan_tags(tag_string)
@@ -788,11 +769,31 @@ class Post < ActiveRecord::Base
end end
end end
module VersionMethods
def create_version
if created_at == updated_at
versions.create(
:rating => rating,
:source => source,
:add_tags => tag_string,
:parent_id => parent_id
)
else
versions.create(
:rating => rating_changed? ? rating : nil,
:source => source_changed? ? source : nil,
:add_tags => (tag_array - tag_array_was).join(" "),
:del_tags => (tag_array_was - tag_array).join(" "),
:parent_id => parent_id_changed? ? parent_id : nil
)
end
end
end
include FileMethods include FileMethods
include ImageMethods include ImageMethods
include ApprovalMethods include ApprovalMethods
include PresenterMethods include PresenterMethods
include HistoryMethods
include TagMethods include TagMethods
include FavoriteMethods include FavoriteMethods
include UploaderMethods include UploaderMethods
@@ -803,6 +804,7 @@ class Post < ActiveRecord::Base
include CacheMethods include CacheMethods
include ParentMethods include ParentMethods
include RemovalMethods include RemovalMethods
include VersionMethods
def reload(options = nil) def reload(options = nil)
super super

View File

@@ -1,114 +0,0 @@
class PostHistory < ActiveRecord::Base
class Error < Exception ; end
class Revision
attr_accessor :prev, :hash, :diff, :tag_array
def initialize(hash)
@hash = hash
@diff = {}
@tag_array = Tag.scan_tags(@hash["tag_string"])
end
def calculate_diff
if prev.nil?
diff[:add] = tag_array
diff[:del] = []
diff[:rating] = rating
diff[:source] = source
diff[:parent_id] = parent_id
else
diff[:del] = prev.tag_array - tag_array
diff[:add] = tag_array - prev.tag_array
if prev.rating != rating
diff[:rating] = rating
end
if prev.source != source
diff[:source] = source
end
if prev.parent_id != parent_id
diff[:parent_id]= parent_id
end
end
end
def rating
hash["rating"]
end
def source
hash["source"]
end
def parent_id
hash["parent_id"]
end
def updated_at
hash["updated_at"]
end
def user_id
hash["user_id"]
end
def presenter
@presenter ||= PostHistoryRevisionPresenter.new(self)
end
end
before_validation :initialize_revisions, :on => :create
belongs_to :post
def self.build_revision_for_post(post)
hash = {
:source => post.source,
:rating => post.rating,
:tag_string => post.tag_string,
:parent_id => post.parent_id,
:user_id => CurrentUser.id,
:ip_addr => CurrentUser.ip_addr,
:updated_at => revision_time
}
end
def self.revision_time
Time.now
end
def initialize_revisions
write_attribute(:revisions, "[]")
end
def revisions
if read_attribute(:revisions).blank?
[]
else
JSON.parse(read_attribute(:revisions))
end
end
def <<(post)
revision = self.class.build_revision_for_post(post)
write_attribute(:revisions, (revisions << revision).to_json)
save
end
def each_revision(&block)
array = revisions.map {|x| Revision.new(x)}
link_revisions(array)
array.each {|x| x.calculate_diff}
array.each(&block)
end
private
def link_revisions(array)
1.upto(array.size - 1) do |i|
array[i].prev = array[i - 1]
end
end
end

View File

@@ -0,0 +1,36 @@
class PostVersion < ActiveRecord::Base
belongs_to :post
belongs_to :updater, :class_name => "User"
before_validation :initialize_updater
def self.create_from_post(post)
if post.created_at == post.updated_at
create_from_created_post(post)
else
create_from_updated_post(post)
end
end
def initialize_updater
self.updater_id = CurrentUser.id
self.updater_ip_addr = CurrentUser.ip_addr
end
def add_tag_array
@add_tag_array ||= add_tags.scan(/\S+/)
end
def del_tag_array
@del_tag_array ||= del_tags.scan(/\S+/)
end
def presenter
PostVersionPresenter.new(self)
end
def reload
@add_tag_array = nil
@del_tag_array = nil
super
end
end

View File

@@ -1,25 +0,0 @@
class PostHistoryRevisionPresenter < Presenter
attr_reader :revision
def initialize(revision)
@revision = revision
end
def changes
html = []
html << revision.diff[:del].map {|x| "<del>#{h(x)}</del>"}
html << revision.diff[:add].map {|x| "<ins>#{h(x)}</ins>"}
html << "<ins>source:#{h(revision.diff[:source])}</ins>" if revision.diff[:source].present?
html << "<ins>rating:#{h(revision.diff[:rating])}</ins>" if revision.diff[:rating].present?
html << "<ins>parent:#{revision.diff[:parent_id]}</ins>" if revision.diff[:parent_id].present?
html.join(" ").html_safe
end
def updated_at
Time.parse(revision.updated_at)
end
def updater_name
User.id_to_name(revision.user_id)
end
end

View File

@@ -0,0 +1,21 @@
class PostVersionPresenter < Presenter
attr_reader :post_version
def initialize(post_version)
@post_version = post_version
end
def changes
html = []
html << post_version.del_tag_array.map {|x| "<del>#{h(x)}</del>"}
html << post_version.add_tag_array.map {|x| "<ins>#{h(x)}</ins>"}
html << "<ins>source:#{h(post_version.source)}</ins>" if post_version.source
html << "<ins>rating:#{h(post_version.rating)}</ins>" if post_version.rating
html << "<ins>parent:#{post_version.parent_id}</ins>" if post_version.parent_id
html.join(" ").html_safe
end
def updater_name
User.id_to_name(post_version.updater_id)
end
end

View File

@@ -1,37 +0,0 @@
<div class="post_histories">
<div class="index">
<h1>Post History</h1>
<% @histories.each do |history| %>
<div class="post">
<div class="preview">
<%= history.post.presenter.preview_html %>
</div>
<div class="history">
<table class="striped">
<thead>
<tr>
<th>Date</th>
<th>Updater</th>
<th>Changes</th>
</tr>
</thead>
<tbody>
<% history.each_revision do |revision| %>
<tr>
<td><%= revision.presenter.updated_at %></td>
<td><%= revision.presenter.updater_name %></td>
<td><%= revision.presenter.changes %></td>
</tr>
<% end %>
</tbody>
</table>
</div>
<div class="clearfix"></div>
</div>
<% end %>
</div>
</div>

View File

@@ -0,0 +1,25 @@
<div class="post_versions">
<div class="index">
<h1>Post Versions</h1>
<table class="striped">
<thead>
<tr>
<th>Date</th>
<th>Updater</th>
<th>Changes</th>
</tr>
</thead>
<tbody>
<% @post_versions.each do |post_version| %>
<tr>
<td><%= time_ago_in_words post_version.updated_at %></td>
<td><%= post_version.presenter.updater_name %></td>
<td><%= post_version.presenter.changes %></td>
</tr>
<% end %>
</tbody>
</table>
</div>
</div>

View File

@@ -48,7 +48,7 @@ Danbooru::Application.routes.draw do
end end
end end
resources :post_histories, :only => [:index] resources :post_versions, :only => [:index]
resource :session resource :session
resources :tags resources :tags
resources :tag_aliases do resources :tag_aliases do

View File

@@ -1046,23 +1046,29 @@ ALTER SEQUENCE post_disapprovals_id_seq OWNED BY post_disapprovals.id;
-- --
-- Name: post_histories; Type: TABLE; Schema: public; Owner: -; Tablespace: -- Name: post_versions; Type: TABLE; Schema: public; Owner: -; Tablespace:
-- --
CREATE TABLE post_histories ( CREATE TABLE post_versions (
id integer NOT NULL, id integer NOT NULL,
created_at timestamp without time zone, created_at timestamp without time zone,
updated_at timestamp without time zone, updated_at timestamp without time zone,
post_id integer NOT NULL, post_id integer NOT NULL,
revisions text NOT NULL add_tags text DEFAULT ''::text NOT NULL,
del_tags text DEFAULT ''::text NOT NULL,
rating character(1),
parent_id integer,
source text,
updater_id integer NOT NULL,
updater_ip_addr inet NOT NULL
); );
-- --
-- Name: post_histories_id_seq; Type: SEQUENCE; Schema: public; Owner: - -- Name: post_versions_id_seq; Type: SEQUENCE; Schema: public; Owner: -
-- --
CREATE SEQUENCE post_histories_id_seq CREATE SEQUENCE post_versions_id_seq
START WITH 1 START WITH 1
INCREMENT BY 1 INCREMENT BY 1
NO MAXVALUE NO MAXVALUE
@@ -1071,10 +1077,10 @@ CREATE SEQUENCE post_histories_id_seq
-- --
-- Name: post_histories_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: - -- Name: post_versions_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: -
-- --
ALTER SEQUENCE post_histories_id_seq OWNED BY post_histories.id; ALTER SEQUENCE post_versions_id_seq OWNED BY post_versions.id;
-- --
@@ -1814,7 +1820,7 @@ ALTER TABLE post_disapprovals ALTER COLUMN id SET DEFAULT nextval('post_disappro
-- Name: id; Type: DEFAULT; Schema: public; Owner: - -- Name: id; Type: DEFAULT; Schema: public; Owner: -
-- --
ALTER TABLE post_histories ALTER COLUMN id SET DEFAULT nextval('post_histories_id_seq'::regclass); ALTER TABLE post_versions ALTER COLUMN id SET DEFAULT nextval('post_versions_id_seq'::regclass);
-- --
@@ -2141,11 +2147,11 @@ ALTER TABLE ONLY post_disapprovals
-- --
-- Name: post_histories_pkey; Type: CONSTRAINT; Schema: public; Owner: -; Tablespace: -- Name: post_versions_pkey; Type: CONSTRAINT; Schema: public; Owner: -; Tablespace:
-- --
ALTER TABLE ONLY post_histories ALTER TABLE ONLY post_versions
ADD CONSTRAINT post_histories_pkey PRIMARY KEY (id); ADD CONSTRAINT post_versions_pkey PRIMARY KEY (id);
-- --
@@ -2722,10 +2728,24 @@ CREATE INDEX index_post_disapprovals_on_user_id ON post_disapprovals USING btree
-- --
-- Name: index_post_histories_on_post_id; Type: INDEX; Schema: public; Owner: -; Tablespace: -- Name: index_post_versions_on_post_id; Type: INDEX; Schema: public; Owner: -; Tablespace:
-- --
CREATE INDEX index_post_histories_on_post_id ON post_histories USING btree (post_id); CREATE INDEX index_post_versions_on_post_id ON post_versions USING btree (post_id);
--
-- Name: index_post_versions_on_updater_id; Type: INDEX; Schema: public; Owner: -; Tablespace:
--
CREATE INDEX index_post_versions_on_updater_id ON post_versions USING btree (updater_id);
--
-- Name: index_post_versions_on_updater_ip_addr; Type: INDEX; Schema: public; Owner: -; Tablespace:
--
CREATE INDEX index_post_versions_on_updater_ip_addr ON post_versions USING btree (updater_ip_addr);
-- --

View File

@@ -1,16 +0,0 @@
class CreatePostHistories < ActiveRecord::Migration
def self.up
create_table :post_histories do |t|
t.timestamps
t.column :post_id, :integer, :null => false
t.column :revisions, :text, :null => false
end
add_index :post_histories, :post_id
end
def self.down
drop_table :post_histories
end
end

View File

@@ -0,0 +1,24 @@
class CreatePostVersions < ActiveRecord::Migration
def self.up
create_table :post_versions do |t|
t.timestamps
t.column :post_id, :integer, :null => false
t.column :add_tags, :text, :null => false, :default => ""
t.column :del_tags, :text, :null => false, :default => ""
t.column :rating, :char
t.column :parent_id, :integer
t.column :source, :text
t.column :updater_id, :integer, :null => false
t.column :updater_ip_addr, "inet", :null => false
end
add_index :post_versions, :post_id
add_index :post_versions, :updater_id
add_index :post_versions, :updater_ip_addr
end
def self.down
drop_table :post_versions
end
end

View File

@@ -1,8 +1,36 @@
require 'test_helper' require 'test_helper'
class PostVersionsControllerTest < ActionController::TestCase class PostVersionsControllerTest < ActionController::TestCase
# Replace this with your real tests. context "The post versions controller" do
test "the truth" do setup do
assert true @user = Factory.create(:user)
CurrentUser.user = @user
CurrentUser.ip_addr = "127.0.0.1"
end
teardown do
CurrentUser.user = nil
CurrentUser.ip_addr = nil
end
context "index action" do
setup do
@post = Factory.create(:post)
@post.update_attributes(:tag_string => "1 2", :source => "xxx")
@post.update_attributes(:tag_string => "2 3", :rating => "e")
end
should "list all versions" do
get :index
assert_response :success
assert_not_nil(assigns(:post_versions))
end
should "list all versions that match the search criteria" do
get :index, {:search => {:post_id_equals => @post.id}}
assert_response :success
assert_not_nil(assigns(:post_versions))
end
end
end end
end end

View File

@@ -1,8 +1,37 @@
require 'test_helper' require 'test_helper'
class PostVotesControllerTest < ActionController::TestCase class PostVotesControllerTest < ActionController::TestCase
# Replace this with your real tests. context "The post vote controller" do
test "the truth" do setup do
assert true @user = Factory.create(:user)
CurrentUser.user = @user
CurrentUser.ip_addr = "127.0.0.1"
@post = Factory.create(:post)
end
teardown do
CurrentUser.user = nil
CurrentUser.ip_addr = nil
end
context "create action" do
should "increment a post's score if the score is positive" do
post :create, {:post_id => @post.id, :score => "up", :format => "js"}, {:user_id => @user.id}
@post.reload
assert_equal(1, @post.score)
end
context "for a post that has already been voted on" do
setup do
@post.vote!("up")
end
should "fail silently on an error" do
assert_nothing_raised do
post :create, {:post_id => @post.id, :score => "up", :format => "js"}, {:user_id => @user.id}
end
end
end
end
end end
end end

View File

@@ -1,63 +0,0 @@
require_relative '../test_helper'
class PostHistoryTest < ActiveSupport::TestCase
context "A post" do
setup do
@user = Factory.create(:user)
CurrentUser.user = @user
CurrentUser.ip_addr = "127.0.0.1"
MEMCACHE.flush_all
PostHistory.stubs(:revision_time).returns("TIME")
end
teardown do
CurrentUser.user = nil
CurrentUser.ip_addr = nil
end
should "create a revision after creation" do
post = Factory.create(:post, :tag_string => "aaa bbb ccc")
assert_equal(1, post.revisions.size)
assert_equal({"source"=>nil, "rating"=>"q", "tag_string"=>"aaa bbb ccc", "parent_id"=>nil, "user_id"=>@user.id, "ip_addr"=>"127.0.0.1", "updated_at"=>"TIME"}, post.revisions.last)
end
should "create additional revisions after updating" do
post = Factory.create(:post, :tag_string => "aaa bbb ccc")
post.update_attributes(:tag_string => "bbb ccc ddd")
post.reload
assert_equal(2, post.revisions.size)
assert_equal({"source"=>nil, "rating"=>"q", "tag_string"=>"bbb ccc ddd", "parent_id"=>nil, "user_id"=>@user.id, "ip_addr"=>"127.0.0.1", "updated_at"=>"TIME"}, post.revisions.last)
end
context "history" do
setup do
@post = Factory.create(:post, :tag_string => "aaa bbb ccc", :source => "xyz", :rating => "q")
@post.update_attributes(:tag_string => "bbb ccc ddd", :source => "abc", :rating => "s")
@post.update_attributes(:tag_string => "ccc ddd eee")
@revisions = []
@post.history.each_revision do |revision|
@revisions << revision
end
end
should "link revisions together" do
assert_nil(@revisions[0].prev)
assert_equal(@revisions[0], @revisions[1].prev)
assert_equal(@revisions[1], @revisions[2].prev)
end
should "iterate over its revisions" do
assert_equal(3, @revisions.size)
assert_equal(%w(aaa bbb ccc), @revisions[0].tag_array)
assert_equal(%w(bbb ccc ddd), @revisions[1].tag_array)
assert_equal(%w(ccc ddd eee), @revisions[2].tag_array)
end
should "create a diff for each revision detailing what changed" do
assert_equal({:add=>["aaa", "bbb", "ccc"], :del=>[], :rating=>"q", :source=>"xyz", :parent_id=>nil}, @revisions[0].diff)
assert_equal({:del=>["aaa"], :add=>["ddd"], :rating=>"s", :source=>"abc"}, @revisions[1].diff)
assert_equal({:del=>["bbb"], :add=>["eee"]}, @revisions[2].diff)
end
end
end
end

View File

@@ -0,0 +1,52 @@
require_relative '../test_helper'
class PostVersionTest < ActiveSupport::TestCase
context "A post" do
setup do
@user = Factory.create(:user)
CurrentUser.user = @user
CurrentUser.ip_addr = "127.0.0.1"
MEMCACHE.flush_all
end
teardown do
CurrentUser.user = nil
CurrentUser.ip_addr = nil
end
context "that has been created" do
setup do
@parent = Factory.create(:post)
@post = Factory.create(:post, :tag_string => "aaa bbb ccc", :rating => "e", :parent => @parent, :source => "xyz")
end
should "also create a version" do
assert_equal(1, @post.versions.size)
@version = @post.versions.last
assert_equal("aaa bbb ccc", @version.add_tags)
assert_equal("", @version.del_tags)
assert_equal(@post.rating, @version.rating)
assert_equal(@post.parent_id, @version.parent_id)
assert_equal(@post.source, @version.source)
end
end
context "that has been updated" do
setup do
@parent = Factory.create(:post)
@post = Factory.create(:post, :tag_string => "aaa bbb ccc", :rating => "q", :source => "xyz")
@post.update_attributes(:tag_string => "bbb ccc xxx", :source => "")
end
should "also create a version" do
assert_equal(2, @post.versions.size)
@version = @post.versions.last
assert_equal("xxx", @version.add_tags)
assert_equal("aaa", @version.del_tags)
assert_nil(@version.rating)
assert_equal("", @version.source)
assert_nil(@version.parent_id)
end
end
end
end

View File

@@ -68,7 +68,7 @@ class TagAliasTest < ActiveSupport::TestCase
ta1 = Factory.create(:tag_alias, :antecedent_name => "aaa", :consequent_name => "xxx") ta1 = Factory.create(:tag_alias, :antecedent_name => "aaa", :consequent_name => "xxx")
p1.reload p1.reload
assert_not_equal("uploader:#{ta1.creator_id}", p1.uploader_string) assert_not_equal("uploader:#{ta1.creator_id}", p1.uploader_string)
assert_equal(ta1.creator_id, p1.history.revisions.last["user_id"]) assert_equal(ta1.creator_id, p1.versions.last.updater_id)
end end
end end
end end

View File

@@ -114,7 +114,7 @@ class TagImplicationTest < ActiveSupport::TestCase
ti1 = Factory.create(:tag_implication, :antecedent_name => "aaa", :consequent_name => "xxx") ti1 = Factory.create(:tag_implication, :antecedent_name => "aaa", :consequent_name => "xxx")
p1.reload p1.reload
assert_not_equal("uploader:#{ti1.creator_id}", p1.uploader_string) assert_not_equal("uploader:#{ti1.creator_id}", p1.uploader_string)
assert_equal(ti1.creator_id, p1.history.revisions.last["user_id"]) assert_equal(ti1.creator_id, p1.versions.last.updater_id)
end end
end end
end end