From 2dbb869188fc72f87a19675494ccff5e03095973 Mon Sep 17 00:00:00 2001 From: Albert Yi Date: Mon, 31 Oct 2016 17:51:44 -0700 Subject: [PATCH] keep track of post approvals to prevent approval cycles --- app/logical/daily_maintenance.rb | 1 + app/models/post.rb | 9 ++- app/models/post_approval.rb | 12 ++++ app/views/posts/show.html.erb | 4 +- .../20161101003139_create_post_approvals.rb | 12 ++++ db/structure.sql | 63 +++++++++++++++++++ test/models/post_approval_test.rb | 62 ++++++++++++++++++ 7 files changed, 159 insertions(+), 4 deletions(-) create mode 100644 app/models/post_approval.rb create mode 100644 db/migrate/20161101003139_create_post_approvals.rb create mode 100644 test/models/post_approval_test.rb diff --git a/app/logical/daily_maintenance.rb b/app/logical/daily_maintenance.rb index 245113536..ece64f4c4 100644 --- a/app/logical/daily_maintenance.rb +++ b/app/logical/daily_maintenance.rb @@ -15,6 +15,7 @@ class DailyMaintenance TagAlias.update_cached_post_counts_for_all PostDisapproval.dmail_messages! Tag.clean_up_negative_post_counts! + PostApproval.prune! SuperVoter.init! AntiVoter.init! end diff --git a/app/models/post.rb b/app/models/post.rb index a114c3b41..edd62ca11 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -274,7 +274,7 @@ class Post < ActiveRecord::Base module ApprovalMethods def is_approvable? - !is_status_locked? && (is_pending? || is_flagged? || is_deleted?) && approver_id != CurrentUser.id + !is_status_locked? && (is_pending? || is_flagged? || is_deleted?) && !PostApproval.approved?(CurrentUser.id, id) end def flag!(reason, options = {}) @@ -314,7 +314,7 @@ class Post < ActiveRecord::Base raise ApprovalError.new("You cannot approve a post you uploaded") end - if approver_id == CurrentUser.id + if approver_id == CurrentUser.id || PostApproval.approved?(CurrentUser.id, id) errors.add(:approver, "have already approved this post") raise ApprovalError.new("You have previously approved this post and cannot approve it again") end @@ -324,6 +324,9 @@ class Post < ActiveRecord::Base self.is_pending = false self.is_deleted = false self.approver_id = CurrentUser.id + + PostApproval.create(user_id: CurrentUser.id, post_id: id) + save! end @@ -1362,7 +1365,7 @@ class Post < ActiveRecord::Base end if !CurrentUser.is_admin? - if approver_id == CurrentUser.id + if approver_id == CurrentUser.id || PostApproval.approved?(CurrentUser.id, id) raise ApprovalError.new("You have previously approved this post and cannot undelete it") elsif uploader_id == CurrentUser.id raise ApprovalError.new("You cannot undelete a post you uploaded") diff --git a/app/models/post_approval.rb b/app/models/post_approval.rb new file mode 100644 index 000000000..395401011 --- /dev/null +++ b/app/models/post_approval.rb @@ -0,0 +1,12 @@ +class PostApproval < ActiveRecord::Base + belongs_to :user + belongs_to :post + + def self.prune! + where("created_at < ?", 1.month.ago).delete_all + end + + def self.approved?(user_id, post_id) + where(user_id: user_id, post_id: post_id).exists? + end +end diff --git a/app/views/posts/show.html.erb b/app/views/posts/show.html.erb index 2d547abf1..a9806fb0d 100644 --- a/app/views/posts/show.html.erb +++ b/app/views/posts/show.html.erb @@ -148,7 +148,9 @@ - + <% if CurrentUser.can_approve_posts? %> + + <% end %> diff --git a/db/migrate/20161101003139_create_post_approvals.rb b/db/migrate/20161101003139_create_post_approvals.rb new file mode 100644 index 000000000..512f62f9f --- /dev/null +++ b/db/migrate/20161101003139_create_post_approvals.rb @@ -0,0 +1,12 @@ +class CreatePostApprovals < ActiveRecord::Migration + def change + create_table :post_approvals do |t| + t.integer :user_id, null: false + t.integer :post_id, null: false + t.timestamps null: false + end + + add_index :post_approvals, :user_id + add_index :post_approvals, :post_id + end +end diff --git a/db/structure.sql b/db/structure.sql index d03ad6b63..c235144b3 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -2617,6 +2617,38 @@ CREATE SEQUENCE post_appeals_id_seq ALTER SEQUENCE post_appeals_id_seq OWNED BY post_appeals.id; +-- +-- Name: post_approvals; Type: TABLE; Schema: public; Owner: - +-- + +CREATE TABLE post_approvals ( + id integer NOT NULL, + user_id integer NOT NULL, + post_id integer NOT NULL, + created_at timestamp without time zone NOT NULL, + updated_at timestamp without time zone NOT NULL +); + + +-- +-- Name: post_approvals_id_seq; Type: SEQUENCE; Schema: public; Owner: - +-- + +CREATE SEQUENCE post_approvals_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + + +-- +-- Name: post_approvals_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: - +-- + +ALTER SEQUENCE post_approvals_id_seq OWNED BY post_approvals.id; + + -- -- Name: post_disapprovals; Type: TABLE; Schema: public; Owner: - -- @@ -4307,6 +4339,13 @@ ALTER TABLE ONLY pools ALTER COLUMN id SET DEFAULT nextval('pools_id_seq'::regcl ALTER TABLE ONLY post_appeals ALTER COLUMN id SET DEFAULT nextval('post_appeals_id_seq'::regclass); +-- +-- Name: id; Type: DEFAULT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY post_approvals ALTER COLUMN id SET DEFAULT nextval('post_approvals_id_seq'::regclass); + + -- -- Name: id; Type: DEFAULT; Schema: public; Owner: - -- @@ -4712,6 +4751,14 @@ ALTER TABLE ONLY post_appeals ADD CONSTRAINT post_appeals_pkey PRIMARY KEY (id); +-- +-- Name: post_approvals_pkey; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY post_approvals + ADD CONSTRAINT post_approvals_pkey PRIMARY KEY (id); + + -- -- Name: post_disapprovals_pkey; Type: CONSTRAINT; Schema: public; Owner: - -- @@ -6768,6 +6815,20 @@ CREATE INDEX index_post_appeals_on_creator_ip_addr ON post_appeals USING btree ( CREATE INDEX index_post_appeals_on_post_id ON post_appeals USING btree (post_id); +-- +-- Name: index_post_approvals_on_post_id; Type: INDEX; Schema: public; Owner: - +-- + +CREATE INDEX index_post_approvals_on_post_id ON post_approvals USING btree (post_id); + + +-- +-- Name: index_post_approvals_on_user_id; Type: INDEX; Schema: public; Owner: - +-- + +CREATE INDEX index_post_approvals_on_user_id ON post_approvals USING btree (user_id); + + -- -- Name: index_post_disapprovals_on_post_id; Type: INDEX; Schema: public; Owner: - -- @@ -7459,3 +7520,5 @@ INSERT INTO schema_migrations (version) VALUES ('20161018221128'); INSERT INTO schema_migrations (version) VALUES ('20161024220345'); +INSERT INTO schema_migrations (version) VALUES ('20161101003139'); + diff --git a/test/models/post_approval_test.rb b/test/models/post_approval_test.rb new file mode 100644 index 000000000..756f5de4a --- /dev/null +++ b/test/models/post_approval_test.rb @@ -0,0 +1,62 @@ +require 'test_helper' + +class PostApprovalTest < ActiveSupport::TestCase + context "a pending post" do + setup do + @user = FactoryGirl.create(:user) + CurrentUser.user = @user + CurrentUser.ip_addr = "127.0.0.1" + + @post = FactoryGirl.create(:post, uploader_id: @user.id, is_pending: true) + + @approver = FactoryGirl.create(:user) + @approver.can_approve_posts = true + @approver.save + CurrentUser.user = @approver + + CurrentUser.stubs(:can_approve_posts?).returns(true) + end + + teardown do + CurrentUser.user = nil + end + + should "allow approval" do + assert_equal(false, PostApproval.approved?(@approver.id, @post.id)) + end + + context "That is approved" do + should "create a postapproval record" do + assert_difference("PostApproval.count") do + @post.approve! + end + end + + context "that is then flagged" do + setup do + @user2 = FactoryGirl.create(:user) + @user3 = FactoryGirl.create(:user) + @approver2 = FactoryGirl.create(:user) + @approver2.can_approve_posts = true + @approver2.save + end + + should "prevent the first approver from approving again" do + @post.approve! + CurrentUser.user = @user2 + @post.flag!("blah") + CurrentUser.user = @approver2 + @post.approve! + assert_not_equal(@approver.id, @post.approver_id) + CurrentUser.user = @user3 + @post.flag!("blah blah") + CurrentUser.user = @approver + + assert_raises(Post::ApprovalError) do + @post.approve! + end + end + end + end + end +end