diff --git a/app/controllers/post_actions_controller.rb b/app/controllers/post_actions_controller.rb
index 1312a22ae..ecda9ffa3 100644
--- a/app/controllers/post_actions_controller.rb
+++ b/app/controllers/post_actions_controller.rb
@@ -4,39 +4,30 @@ class PostActionsController < ApplicationController
before_filter :ensure_logged_in, except: :users
before_filter :fetch_post_from_params
+ before_filter :fetch_post_action_type_id_from_params
def create
- id = params[:post_action_type_id].to_i
- if action = PostActionType.where(id: id).first
- guardian.ensure_post_can_act!(@post, PostActionType.types[id])
+ guardian.ensure_post_can_act!(@post, PostActionType.types[@post_action_type_id])
- post_action = PostAction.act(current_user, @post, action.id, params[:message])
-
- if post_action.blank? || post_action.errors.present?
- render_json_error(post_action)
- else
- # We need to reload or otherwise we are showing the old values on the front end
- @post.reload
- post_serializer = PostSerializer.new(@post, scope: guardian, root: false)
- render_json_dump(post_serializer)
- end
+ post_action = PostAction.act(current_user, @post, @post_action_type_id, params[:message])
+ if post_action.blank? || post_action.errors.present?
+ render_json_error(post_action)
else
- raise Discourse::InvalidParameters.new(:post_action_type_id)
+ # We need to reload or otherwise we are showing the old values on the front end
+ @post.reload
+ post_serializer = PostSerializer.new(@post, scope: guardian, root: false)
+ render_json_dump(post_serializer)
end
end
def users
- requires_parameter(:post_action_type_id)
- post_action_type_id = params[:post_action_type_id].to_i
-
- guardian.ensure_can_see_post_actors!(@post.topic, post_action_type_id)
-
- users = User.
- select(['null as post_url','users.id', 'users.username', 'users.username_lower', 'users.email','post_actions.related_post_id']).
- joins(:post_actions).
- where(['post_actions.post_id = ? and post_actions.post_action_type_id = ? and post_actions.deleted_at IS NULL', @post.id, post_action_type_id]).all
+ guardian.ensure_can_see_post_actors!(@post.topic, @post_action_type_id)
+ users = User.select(['null as post_url','users.id', 'users.username', 'users.username_lower', 'users.email','post_actions.related_post_id'])
+ .joins(:post_actions)
+ .where(['post_actions.post_id = ? and post_actions.post_action_type_id = ? and post_actions.deleted_at IS NULL', @post.id, @post_action_type_id])
+ .all
urls = Post.urls(users.map{|u| u.related_post_id})
users.each do |u|
@@ -47,21 +38,21 @@ class PostActionsController < ApplicationController
end
def destroy
- requires_parameter(:post_action_type_id)
+ post_action = current_user.post_actions.where(post_id: params[:id].to_i, post_action_type_id: @post_action_type_id, deleted_at: nil).first
- post_action = current_user.post_actions.where(post_id: params[:id].to_i, post_action_type_id: params[:post_action_type_id].to_i, deleted_at: nil).first
raise Discourse::NotFound if post_action.blank?
+
guardian.ensure_can_delete!(post_action)
+
PostAction.remove_act(current_user, @post, post_action.post_action_type_id)
render nothing: true
end
def clear_flags
- requires_parameter(:post_action_type_id)
guardian.ensure_can_clear_flags!(@post)
- PostAction.clear_flags!(@post, current_user.id, params[:post_action_type_id].to_i)
+ PostAction.clear_flags!(@post, current_user.id, @post_action_type_id)
@post.reload
if @post.is_flagged?
@@ -84,4 +75,9 @@ class PostActionsController < ApplicationController
@post = finder.first
guardian.ensure_can_see!(@post)
end
+
+ def fetch_post_action_type_id_from_params
+ requires_parameter(:post_action_type_id)
+ @post_action_type_id = params[:post_action_type_id].to_i
+ end
end
diff --git a/app/models/post_action.rb b/app/models/post_action.rb
index 789d33c0f..0167ad389 100644
--- a/app/models/post_action.rb
+++ b/app/models/post_action.rb
@@ -2,7 +2,7 @@ require_dependency 'rate_limiter'
require_dependency 'system_message'
class PostAction < ActiveRecord::Base
- class AlreadyFlagged < StandardError; end
+ class AlreadyActed < StandardError; end
include RateLimiter::OnCreateRecord
@@ -22,7 +22,8 @@ class PostAction < ActiveRecord::Base
posts_flagged_count = PostAction.joins(post: :topic)
.where('post_actions.post_action_type_id' => PostActionType.notify_flag_types.values,
'posts.deleted_at' => nil,
- 'topics.deleted_at' => nil).count('DISTINCT posts.id')
+ 'topics.deleted_at' => nil)
+ .count('DISTINCT posts.id')
$redis.set('posts_flagged_count', posts_flagged_count)
user_ids = User.staff.select(:id).map {|u| u.id}
@@ -156,9 +157,12 @@ class PostAction < ActiveRecord::Base
end
before_create do
- raise AlreadyFlagged if is_flag? && PostAction.where(user_id: user_id,
- post_id: post_id,
- post_action_type_id: PostActionType.flag_types.values).exists?
+ post_action_type_ids = is_flag? ? PostActionType.flag_types.values : post_action_type_id
+ raise AlreadyActed if PostAction.where(user_id: user_id,
+ post_id: post_id,
+ post_action_type_id: post_action_type_ids,
+ deleted_at: nil)
+ .exists?
end
after_save do
diff --git a/spec/models/post_action_spec.rb b/spec/models/post_action_spec.rb
index 7a341ea93..980d75a5f 100644
--- a/spec/models/post_action_spec.rb
+++ b/spec/models/post_action_spec.rb
@@ -19,15 +19,14 @@ describe PostAction do
describe "messaging" do
- it "notify moderators integration test" do
+ it "notify moderators integration test" do
mod = moderator
action = PostAction.act(codinghorror, post, PostActionType.types[:notify_moderators], "this is my special long message");
- posts = Post.
- joins(:topic).
- select('posts.id, topics.subtype').
- where('topics.archetype' => Archetype.private_message).
- to_a
+ posts = Post.joins(:topic)
+ .select('posts.id, topics.subtype')
+ .where('topics.archetype' => Archetype.private_message)
+ .to_a
posts.count.should == 1
action.related_post_id.should == posts[0].id.to_i
@@ -85,14 +84,35 @@ describe PostAction do
end
- it "increases the post's bookmark count when saved" do
- lambda { bookmark.save; post.reload }.should change(post, :bookmark_count).by(1)
- end
+ describe "when a user bookmarks something" do
+ it "increases the post's bookmark count when saved" do
+ lambda { bookmark.save; post.reload }.should change(post, :bookmark_count).by(1)
+ end
- it "increases the forum topic's bookmark count when saved" do
- lambda { bookmark.save; post.topic.reload }.should change(post.topic, :bookmark_count).by(1)
- end
+ it "increases the forum topic's bookmark count when saved" do
+ lambda { bookmark.save; post.topic.reload }.should change(post.topic, :bookmark_count).by(1)
+ end
+ describe 'when deleted' do
+
+ before do
+ bookmark.save
+ post.reload
+ @topic = post.topic
+ @topic.reload
+ bookmark.deleted_at = DateTime.now
+ bookmark.save
+ end
+
+ it 'reduces the bookmark count of the post' do
+ lambda { post.reload }.should change(post, :bookmark_count).by(-1)
+ end
+
+ it 'reduces the bookmark count of the forum topic' do
+ lambda { @topic.reload }.should change(post.topic, :bookmark_count).by(-1)
+ end
+ end
+ end
describe 'when a user likes something' do
it 'should increase the post counts when a user likes' do
@@ -108,11 +128,10 @@ describe PostAction do
post.topic.reload
}.should change(post.topic, :like_count).by(1)
end
-
end
describe 'when a user votes for something' do
- it 'should increase the vote counts when a user likes' do
+ it 'should increase the vote counts when a user votes' do
lambda {
PostAction.act(codinghorror, post, PostActionType.types[:vote])
post.reload
@@ -127,37 +146,21 @@ describe PostAction do
end
end
-
- describe 'when deleted' do
- before do
- bookmark.save
- post.reload
- @topic = post.topic
- @topic.reload
- bookmark.deleted_at = DateTime.now
- bookmark.save
- end
-
- it 'reduces the bookmark count of the post' do
- lambda {
- post.reload
- }.should change(post, :bookmark_count).by(-1)
- end
-
- it 'reduces the bookmark count of the forum topic' do
- lambda {
- @topic.reload
- }.should change(post.topic, :bookmark_count).by(-1)
- end
- end
-
describe 'flagging' do
it 'does not allow you to flag stuff with 2 reasons' do
post = Fabricate(:post)
u1 = Fabricate(:evil_trout)
PostAction.act(u1, post, PostActionType.types[:spam])
- lambda { PostAction.act(u1, post, PostActionType.types[:off_topic]) }.should raise_error(PostAction::AlreadyFlagged)
+ lambda { PostAction.act(u1, post, PostActionType.types[:off_topic]) }.should raise_error(PostAction::AlreadyActed)
+ end
+
+ it 'allows you to flag stuff with another reason' do
+ post = Fabricate(:post)
+ u1 = Fabricate(:evil_trout)
+ PostAction.act(u1, post, PostActionType.types[:spam])
+ PostAction.remove_act(u1, post, PostActionType.types[:spam])
+ lambda { PostAction.act(u1, post, PostActionType.types[:off_topic]) }.should_not raise_error(PostAction::AlreadyActed)
end
it 'should update counts when you clear flags' do
@@ -175,13 +178,10 @@ describe PostAction do
end
it 'should follow the rules for automatic hiding workflow' do
-
post = Fabricate(:post)
u1 = Fabricate(:evil_trout)
u2 = Fabricate(:walter_white)
-
- # we need an admin for the messages
- admin = Fabricate(:admin)
+ admin = Fabricate(:admin) # we need an admin for the messages
SiteSetting.flags_required_to_hide_post = 2
@@ -218,4 +218,18 @@ describe PostAction do
end
end
+ it "prevents user to act twice at the same time" do
+ post = Fabricate(:post)
+ user = Fabricate(:evil_trout)
+
+ # flags are already being tested
+ all_types_except_flags = PostActionType.types.except(PostActionType.flag_types)
+ all_types_except_flags.values.each do |action|
+ lambda do
+ PostAction.act(user, post, action)
+ PostAction.act(user, post, action)
+ end.should raise_error(PostAction::AlreadyActed)
+ end
+ end
+
end
diff --git a/spec/models/post_action_type_spec.rb b/spec/models/post_action_type_spec.rb
deleted file mode 100644
index 8736c1edd..000000000
--- a/spec/models/post_action_type_spec.rb
+++ /dev/null
@@ -1,5 +0,0 @@
-require 'spec_helper'
-
-describe PostActionType do
-
-end