FIX: release/reclaim votes on moving topics to different category #34
Conversation
spec/voting_spec.rb
Outdated
|
|
||
| it "enqueus a job to reclaim votes if voting is enabled for the new category" do | ||
| PostRevisor.new(post1).revise!(admin, category_id: category1.id) | ||
| expect(Jobs::VoteReclaim.jobs.first["args"].first["topic_id"]).to eq(post1.reload.topic_id) |
There was a problem hiding this comment.
A problem with this style of test is that it does not properly test if the thing works, the job may fire and crash for all you know... the better way is just to test that votes actually get reclaimed.
Can you change that?
There was a problem hiding this comment.
Done, let me what you think :D
plugin.rb
Outdated
| @@ -274,6 +274,17 @@ def execute(args) | |||
| end | |||
| end | |||
|
|
|||
| DiscourseEvent.on(:post_edited) do |post, topic_changed| | |||
| if topic_changed && SiteSetting.voting_enabled | |||
There was a problem hiding this comment.
before queuing the job can you do quick check on the topic to see if it has more than 0 votes?
also I guess this leave a whole around bulk actions, does that fire a post edited? (select multiple topics from web and then change category)
There was a problem hiding this comment.
Ok, I've made it so that it doesn't enqueue a job unless there are votes to release or reclaim.
And yes this event doesn't trigger when moving topics in bulk because we don't use PostRevisor there. Should I simply trigger the event or make bulk actions user PostRevisor?
| @@ -274,6 +274,17 @@ def execute(args) | |||
| end | |||
| end | |||
|
|
|||
| DiscourseEvent.on(:post_edited) do |post, topic_changed| | |||
| if topic_changed && SiteSetting.voting_enabled && UserCustomField.where(value: post.topic_id).where("name = 'votes_archive' OR name = 'votes'").exists? | |||
There was a problem hiding this comment.
.where("value = ? AND name in ('votes', 'votes_archive')", post.topic_id)
we should do a follow up PR to introduce a const for "votes" and "votes_archive" cause it is somewhat not nice
…scourse#34) * FIX: release/reclaim votes on moving topics to different category * test jobs * don't enqueue job if there are no votes
No description provided.