diff --git a/app/controllers/post_histories_controller.rb b/app/controllers/post_histories_controller.rb deleted file mode 100644 index a4f0055ad..000000000 --- a/app/controllers/post_histories_controller.rb +++ /dev/null @@ -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 diff --git a/app/controllers/post_versions_controller.rb b/app/controllers/post_versions_controller.rb new file mode 100644 index 000000000..21e63bd4b --- /dev/null +++ b/app/controllers/post_versions_controller.rb @@ -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 diff --git a/app/controllers/post_votes_controller.rb b/app/controllers/post_votes_controller.rb index f64ac0eef..a8e407f06 100644 --- a/app/controllers/post_votes_controller.rb +++ b/app/controllers/post_votes_controller.rb @@ -5,7 +5,4 @@ class PostVotesController < ApplicationController rescue PostVote::Error => x @error = x end - - def destroy - end end diff --git a/app/models/janitor_trial.rb b/app/models/janitor_trial.rb index 7bc988bcb..63b2f3e7a 100644 --- a/app/models/janitor_trial.rb +++ b/app/models/janitor_trial.rb @@ -4,6 +4,11 @@ class JanitorTrial < ActiveRecord::Base after_create :promote_user after_destroy :create_feedback validates_presence_of :user + before_validation :initialize_creator + + def initialize_creator + self.creator_id = CurrentUser.id + end 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." diff --git a/app/models/post.rb b/app/models/post.rb index abd0f932a..315cd8a56 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -3,7 +3,7 @@ class Post < ActiveRecord::Base attr_accessor :old_tag_string, :old_parent_id after_destroy :delete_files - after_save :update_history + after_save :create_version after_save :update_parent_on_save before_save :merge_old_tags before_save :normalize_tags @@ -16,8 +16,7 @@ class Post < ActiveRecord::Base belongs_to :parent, :class_name => "Post" has_one :unapproval, :dependent => :destroy has_one :upload, :dependent => :destroy - has_one :moderation_detail, :class_name => "PostModerationDetail", :dependent => :destroy - has_one :history, :class_name => "PostHistory" + has_many :versions, :class_name => "PostVersion", :dependent => :destroy has_many :votes, :class_name => "PostVote", :dependent => :destroy has_many :notes, :dependent => :destroy has_many :comments @@ -240,24 +239,6 @@ class Post < ActiveRecord::Base 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 def tag_array @tag_array ||= Tag.scan_tags(tag_string) @@ -788,11 +769,31 @@ class Post < ActiveRecord::Base 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 ImageMethods include ApprovalMethods include PresenterMethods - include HistoryMethods include TagMethods include FavoriteMethods include UploaderMethods @@ -803,6 +804,7 @@ class Post < ActiveRecord::Base include CacheMethods include ParentMethods include RemovalMethods + include VersionMethods def reload(options = nil) super diff --git a/app/models/post_history.rb b/app/models/post_history.rb deleted file mode 100644 index e0b899f35..000000000 --- a/app/models/post_history.rb +++ /dev/null @@ -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 diff --git a/app/models/post_version.rb b/app/models/post_version.rb new file mode 100644 index 000000000..4c2ab2fac --- /dev/null +++ b/app/models/post_version.rb @@ -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 diff --git a/app/presenters/post_history_revision_presenter.rb b/app/presenters/post_history_revision_presenter.rb deleted file mode 100644 index c2defe7ac..000000000 --- a/app/presenters/post_history_revision_presenter.rb +++ /dev/null @@ -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| "#{h(x)}"} - html << revision.diff[:add].map {|x| "#{h(x)}"} - html << "source:#{h(revision.diff[:source])}" if revision.diff[:source].present? - html << "rating:#{h(revision.diff[:rating])}" if revision.diff[:rating].present? - html << "parent:#{revision.diff[:parent_id]}" 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 diff --git a/app/presenters/post_version_presenter.rb b/app/presenters/post_version_presenter.rb new file mode 100644 index 000000000..4170fa9e9 --- /dev/null +++ b/app/presenters/post_version_presenter.rb @@ -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| "#{h(x)}"} + html << post_version.add_tag_array.map {|x| "#{h(x)}"} + html << "source:#{h(post_version.source)}" if post_version.source + html << "rating:#{h(post_version.rating)}" if post_version.rating + html << "parent:#{post_version.parent_id}" if post_version.parent_id + html.join(" ").html_safe + end + + def updater_name + User.id_to_name(post_version.updater_id) + end +end diff --git a/app/views/post_histories/index.html.erb b/app/views/post_histories/index.html.erb deleted file mode 100644 index e4015e144..000000000 --- a/app/views/post_histories/index.html.erb +++ /dev/null @@ -1,37 +0,0 @@ -
-
-

Post History

- - <% @histories.each do |history| %> -
-
- <%= history.post.presenter.preview_html %> -
- -
- - - - - - - - - - <% history.each_revision do |revision| %> - - - - - - <% end %> - -
DateUpdaterChanges
<%= revision.presenter.updated_at %><%= revision.presenter.updater_name %><%= revision.presenter.changes %>
-
- -
-
- <% end %> -
-
- diff --git a/app/views/post_versions/index.html.erb b/app/views/post_versions/index.html.erb new file mode 100644 index 000000000..ea839b845 --- /dev/null +++ b/app/views/post_versions/index.html.erb @@ -0,0 +1,25 @@ +
+
+

Post Versions

+ + + + + + + + + + + <% @post_versions.each do |post_version| %> + + + + + + <% end %> + +
DateUpdaterChanges
<%= time_ago_in_words post_version.updated_at %><%= post_version.presenter.updater_name %><%= post_version.presenter.changes %>
+
+
+ diff --git a/config/routes.rb b/config/routes.rb index 2a3917a76..1a39ad1a0 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -48,7 +48,7 @@ Danbooru::Application.routes.draw do end end - resources :post_histories, :only => [:index] + resources :post_versions, :only => [:index] resource :session resources :tags resources :tag_aliases do diff --git a/db/development_structure.sql b/db/development_structure.sql index 2314ce822..2c5389b9d 100644 --- a/db/development_structure.sql +++ b/db/development_structure.sql @@ -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, created_at timestamp without time zone, updated_at timestamp without time zone, 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 INCREMENT BY 1 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: - -- -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 - ADD CONSTRAINT post_histories_pkey PRIMARY KEY (id); +ALTER TABLE ONLY post_versions + 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); -- diff --git a/db/migrate/20100205163027_create_post_histories.rb b/db/migrate/20100205163027_create_post_histories.rb deleted file mode 100644 index 6ed171978..000000000 --- a/db/migrate/20100205163027_create_post_histories.rb +++ /dev/null @@ -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 diff --git a/db/migrate/20100205163027_create_post_versions.rb b/db/migrate/20100205163027_create_post_versions.rb new file mode 100644 index 000000000..d764004ce --- /dev/null +++ b/db/migrate/20100205163027_create_post_versions.rb @@ -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 diff --git a/test/functional/post_versions_controller_test.rb b/test/functional/post_versions_controller_test.rb index 3c9bd9f6d..766974201 100644 --- a/test/functional/post_versions_controller_test.rb +++ b/test/functional/post_versions_controller_test.rb @@ -1,8 +1,36 @@ require 'test_helper' class PostVersionsControllerTest < ActionController::TestCase - # Replace this with your real tests. - test "the truth" do - assert true + context "The post versions controller" do + setup do + @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 diff --git a/test/functional/post_votes_controller_test.rb b/test/functional/post_votes_controller_test.rb index 1bee1e608..20c542751 100644 --- a/test/functional/post_votes_controller_test.rb +++ b/test/functional/post_votes_controller_test.rb @@ -1,8 +1,37 @@ require 'test_helper' class PostVotesControllerTest < ActionController::TestCase - # Replace this with your real tests. - test "the truth" do - assert true + context "The post vote controller" do + setup do + @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 diff --git a/test/unit/post_history_test.rb b/test/unit/post_history_test.rb deleted file mode 100644 index 40d5dd90a..000000000 --- a/test/unit/post_history_test.rb +++ /dev/null @@ -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 diff --git a/test/unit/post_version_test.rb b/test/unit/post_version_test.rb new file mode 100644 index 000000000..5530051fd --- /dev/null +++ b/test/unit/post_version_test.rb @@ -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 diff --git a/test/unit/tag_alias_test.rb b/test/unit/tag_alias_test.rb index b30cb715a..d4a148e29 100644 --- a/test/unit/tag_alias_test.rb +++ b/test/unit/tag_alias_test.rb @@ -68,7 +68,7 @@ class TagAliasTest < ActiveSupport::TestCase ta1 = Factory.create(:tag_alias, :antecedent_name => "aaa", :consequent_name => "xxx") p1.reload 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 diff --git a/test/unit/tag_implication_test.rb b/test/unit/tag_implication_test.rb index 3a2520d41..37153fa22 100644 --- a/test/unit/tag_implication_test.rb +++ b/test/unit/tag_implication_test.rb @@ -114,7 +114,7 @@ class TagImplicationTest < ActiveSupport::TestCase ti1 = Factory.create(:tag_implication, :antecedent_name => "aaa", :consequent_name => "xxx") p1.reload 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