Commit 16929d88184f91e1c85f26d373dfea7cd8608ae9
Exists in
master
and in
7 other branches
Merge branch 'ranking_refactor'
Showing
12 changed files
with
214 additions
and
8 deletions
Show diff stats
... | ... | @@ -0,0 +1,25 @@ |
1 | +class CreateRankingItemTable < ActiveRecord::Migration | |
2 | + | |
3 | + def self.up | |
4 | + create_table :proposals_discussion_plugin_ranking_items do |t| | |
5 | + t.integer :position | |
6 | + t.string :abstract | |
7 | + t.integer :votes_for | |
8 | + t.integer :votes_against | |
9 | + t.integer :hits | |
10 | + t.decimal :effective_support | |
11 | + t.integer :proposal_id | |
12 | + t.timestamps | |
13 | + end | |
14 | + add_index( | |
15 | + :proposals_discussion_plugin_ranking_items, | |
16 | + [:proposal_id], | |
17 | + name: 'index_proposals_discussion_plugin_ranking_proposal_id' | |
18 | + ) | |
19 | + end | |
20 | + | |
21 | + def self.down | |
22 | + drop_table :proposals_discussion_plugin_ranking_items | |
23 | + end | |
24 | + | |
25 | +end | ... | ... |
... | ... | @@ -0,0 +1,13 @@ |
1 | +module Noosfero | |
2 | + module API | |
3 | + module Entities | |
4 | + | |
5 | + class ArticleBase < Entity | |
6 | + expose :ranking_position do |article, options| | |
7 | + article.kind_of?(ProposalsDiscussionPlugin::Proposal) && article.ranking_item.present? ? article.ranking_item.position : nil | |
8 | + end | |
9 | + end | |
10 | + | |
11 | + end | |
12 | + end | |
13 | +end | ... | ... |
lib/proposals_discussion_plugin.rb
lib/proposals_discussion_plugin/api.rb
... | ... | @@ -5,11 +5,9 @@ class ProposalsDiscussionPlugin::API < Grape::API |
5 | 5 | paginate per_page: 10, max_per_page: 20 |
6 | 6 | get ':id/ranking' do |
7 | 7 | article = find_article(environment.articles, params[:id]) |
8 | - ranking = Rails.cache.fetch("#{article.cache_key}/proposals_ranking", expires_in: 30.minutes) do | |
9 | - {:proposals => article.ranking, :updated_at => DateTime.now} | |
10 | - end | |
11 | - ranking[:proposals] = paginate ranking[:proposals] | |
12 | - ranking | |
8 | + current_page = paginate(article.ranking) | |
9 | + #FIXME find a better way to get updated_at date | |
10 | + {:proposals => current_page, :updated_at => current_page.blank? ? DateTime.now : current_page.first.updated_at} | |
13 | 11 | end |
14 | 12 | |
15 | 13 | post ':id/propose' do | ... | ... |
lib/proposals_discussion_plugin/proposal.rb
... | ... | @@ -7,6 +7,8 @@ class ProposalsDiscussionPlugin::Proposal < TinyMceArticle |
7 | 7 | |
8 | 8 | has_many :locations, :class_name => 'Region', :through => :article_categorizations, :source => :category |
9 | 9 | |
10 | + has_one :ranking_item | |
11 | + | |
10 | 12 | def self.short_description |
11 | 13 | _("Proposal") |
12 | 14 | end | ... | ... |
lib/proposals_discussion_plugin/proposals_holder.rb
... | ... | @@ -41,6 +41,10 @@ class ProposalsDiscussionPlugin::ProposalsHolder < Folder |
41 | 41 | end |
42 | 42 | |
43 | 43 | def ranking |
44 | + ProposalsDiscussionPlugin::RankingItem.joins(:proposal => :parent).where('parents_articles.id' => self.id) | |
45 | + end | |
46 | + | |
47 | + def compute_ranking | |
44 | 48 | max_hits = proposals.maximum(:hits) |
45 | 49 | min_hits = proposals.minimum(:hits) |
46 | 50 | |
... | ... | @@ -48,9 +52,18 @@ class ProposalsDiscussionPlugin::ProposalsHolder < Folder |
48 | 52 | w = [(proposal.hits - max_hits).abs, (proposal.hits - min_hits).abs, 1].max.to_f |
49 | 53 | effective_support = (proposal.votes_for - proposal.votes_against)/w |
50 | 54 | |
51 | - {:id => proposal.id, :abstract => proposal.abstract, :votes_for => proposal.votes_for, :votes_against => proposal.votes_against, :hits => proposal.hits, :effective_support => effective_support} | |
55 | + ProposalsDiscussionPlugin::RankingItem.new(:proposal => proposal, :abstract => proposal.abstract, :votes_for => proposal.votes_for, :votes_against => proposal.votes_against, :hits => proposal.hits, :effective_support => effective_support) | |
56 | + end | |
57 | + ranking.sort_by { |p| p.effective_support }.reverse | |
58 | + ranking.each_with_index { |p, i| p.position = i+1 } | |
59 | + end | |
60 | + | |
61 | + def update_ranking | |
62 | + new_ranking = compute_ranking | |
63 | + transaction do | |
64 | + self.ranking.destroy_all | |
65 | + new_ranking.each {|item| item.save!} | |
52 | 66 | end |
53 | - ranking.sort_by { |p| p[:effective_support] }.reverse | |
54 | 67 | end |
55 | 68 | |
56 | 69 | def cache_key_with_person(params = {}, user = nil, language = 'en') | ... | ... |
... | ... | @@ -0,0 +1,36 @@ |
1 | +class ProposalsDiscussionPlugin::RankingJob | |
2 | + | |
3 | + def perform | |
4 | + ProposalsDiscussionPlugin::Topic.find_each do |topic| | |
5 | + ProposalsDiscussionPlugin::RankingJob::TopicRankingJob.new(topic.id).schedule | |
6 | + end | |
7 | + schedule(30.minutes.from_now) | |
8 | + end | |
9 | + | |
10 | + def schedule(run_at = nil) | |
11 | + Delayed::Job.enqueue(self, {:run_at => run_at}) unless self.class.find_job.exists? | |
12 | + end | |
13 | + | |
14 | + def self.find_job | |
15 | + Delayed::Job.by_handler("--- !ruby/object:ProposalsDiscussionPlugin::RankingJob {}\n") | |
16 | + end | |
17 | + | |
18 | + | |
19 | + class TopicRankingJob < Struct.new(:topic_id) | |
20 | + | |
21 | + def perform | |
22 | + topic = ProposalsDiscussionPlugin::Topic.find_by_id(topic_id) | |
23 | + topic.update_ranking if topic.present? | |
24 | + end | |
25 | + | |
26 | + def schedule | |
27 | + Delayed::Job.enqueue(self) unless find_job.exists? | |
28 | + end | |
29 | + | |
30 | + def find_job | |
31 | + Delayed::Job.by_handler("--- !ruby/struct:ProposalsDiscussionPlugin::RankingJob::TopicRankingJob\ntopic_id: #{topic_id}\n") | |
32 | + end | |
33 | + | |
34 | + end | |
35 | + | |
36 | +end | ... | ... |
test/unit/api_test.rb
... | ... | @@ -30,10 +30,12 @@ class APITest < ActiveSupport::TestCase |
30 | 30 | 2.times { Vote.create!(:voteable => proposal3, :voter => nil, :vote => 1) } |
31 | 31 | |
32 | 32 | proposal1.update_attribute(:hits, 5) |
33 | + process_delayed_job_queue | |
33 | 34 | |
34 | 35 | get "/api/v1/proposals_discussion_plugin/#{topic.id}/ranking?#{params.to_query}" |
35 | 36 | json = JSON.parse(last_response.body) |
36 | - assert_equal [proposal2.id, proposal3.id, proposal1.id], json['proposals'].map {|p| p['id']} | |
37 | + assert_equal [proposal2.abstract, proposal3.abstract, proposal1.abstract], json['proposals'].map {|p| p['abstract']} | |
38 | + assert json['updated_at'].to_datetime <= Time.now | |
37 | 39 | end |
38 | 40 | |
39 | 41 | should 'suggest article children' do |
... | ... | @@ -65,4 +67,16 @@ class APITest < ActiveSupport::TestCase |
65 | 67 | assert_equal "This is a malicious body SearchParam", task.article.body |
66 | 68 | end |
67 | 69 | |
70 | + should 'return article position when list proposals' do | |
71 | + discussion = fast_create(ProposalsDiscussionPlugin::Discussion, :profile_id => user.person.id) | |
72 | + topic = fast_create(ProposalsDiscussionPlugin::Topic, :profile_id => user.person.id, :parent_id => discussion.id) | |
73 | + proposal = fast_create(ProposalsDiscussionPlugin::Proposal, :profile_id => user.person.id, :parent_id => topic.id) | |
74 | + params[:content_type] = 'ProposalsDiscussionPlugin::Proposal' | |
75 | + topic.update_ranking | |
76 | + | |
77 | + get "/api/v1/articles/?#{params.to_query}" | |
78 | + json = JSON.parse(last_response.body) | |
79 | + assert_includes json["articles"].map { |a| a["ranking_position"] }, 1 | |
80 | + end | |
81 | + | |
68 | 82 | end | ... | ... |
... | ... | @@ -0,0 +1,19 @@ |
1 | +require_relative '../test_helper' | |
2 | + | |
3 | +class RankingItemTest < ActiveSupport::TestCase | |
4 | + | |
5 | + def setup | |
6 | + @profile = fast_create(Community) | |
7 | + @person = fast_create(Person) | |
8 | + @discussion = ProposalsDiscussionPlugin::Discussion.create!(:name => 'discussion', :profile => person, :allow_topics => false) | |
9 | + end | |
10 | + | |
11 | + attr_reader :profile, :person, :discussion | |
12 | + | |
13 | + should 'return associated ranking item in proposal' do | |
14 | + proposal = ProposalsDiscussionPlugin::Proposal.create!(:name => 'test', :abstract => 'abstract', :profile => profile, :parent => discussion) | |
15 | + discussion.update_ranking | |
16 | + assert proposal.ranking_item | |
17 | + end | |
18 | + | |
19 | +end | ... | ... |
... | ... | @@ -0,0 +1,56 @@ |
1 | +require_relative '../test_helper' | |
2 | + | |
3 | +class RankingJobTest < ActiveSupport::TestCase | |
4 | + | |
5 | + def setup | |
6 | + @job = ProposalsDiscussionPlugin::RankingJob.new | |
7 | + @topic = fast_create(ProposalsDiscussionPlugin::Topic) | |
8 | + @proposal = fast_create(ProposalsDiscussionPlugin::Proposal, :parent_id => topic.id) | |
9 | + end | |
10 | + | |
11 | + attr_accessor :job, :topic, :proposal | |
12 | + | |
13 | + should 'create ranking job in initialization' do | |
14 | + assert job.class.find_job.exists? | |
15 | + end | |
16 | + | |
17 | + should 'do not create duplicated ranking job' do | |
18 | + job.schedule | |
19 | + job.schedule | |
20 | + assert_equal 1, job.class.find_job.count | |
21 | + end | |
22 | + | |
23 | + should 'schedule topic jobs when performed' do | |
24 | + job.perform | |
25 | + assert ProposalsDiscussionPlugin::RankingJob::TopicRankingJob.new(topic.id).find_job.exists? | |
26 | + end | |
27 | + | |
28 | + should 'reschedule job when performed' do | |
29 | + process_delayed_job_queue | |
30 | + job.perform | |
31 | + new_job = job.class.find_job.first | |
32 | + assert new_job.present? | |
33 | + assert new_job.run_at > 20.minutes.from_now | |
34 | + end | |
35 | + | |
36 | + should 'schedule topic job' do | |
37 | + topic_job = ProposalsDiscussionPlugin::RankingJob::TopicRankingJob.new(topic.id) | |
38 | + topic_job.schedule | |
39 | + assert topic_job.find_job.exists? | |
40 | + end | |
41 | + | |
42 | + should 'do not schedule duplicated topic job' do | |
43 | + topic_job = ProposalsDiscussionPlugin::RankingJob::TopicRankingJob.new(topic.id) | |
44 | + topic_job.schedule | |
45 | + topic_job.schedule | |
46 | + assert_equal 1, topic_job.find_job.count | |
47 | + end | |
48 | + | |
49 | + should 'perform topic job' do | |
50 | + job.schedule | |
51 | + assert_equal 0, topic.ranking.count | |
52 | + process_delayed_job_queue | |
53 | + assert_equal 1, topic.ranking.count | |
54 | + end | |
55 | + | |
56 | +end | ... | ... |
test/unit/topic_test.rb
... | ... | @@ -62,4 +62,24 @@ class TopicTest < ActiveSupport::TestCase |
62 | 62 | assert_equal 10, topic.max_score |
63 | 63 | end |
64 | 64 | |
65 | + should 'generate ranking for topics' do | |
66 | + topic2 = ProposalsDiscussionPlugin::Topic.new(:name => 'test2', :profile => @profile, :parent => @discussion) | |
67 | + proposal1 = ProposalsDiscussionPlugin::Proposal.create!(:parent => topic, :profile => profile, :name => "proposal1", :abstract => 'abstract') | |
68 | + proposal2 = ProposalsDiscussionPlugin::Proposal.create!(:parent => topic, :profile => profile, :name => "proposal2", :abstract => 'abstract') | |
69 | + proposal3 = ProposalsDiscussionPlugin::Proposal.create!(:parent => topic2, :profile => profile, :name => "proposal3", :abstract => 'abstract') | |
70 | + | |
71 | + topic.update_ranking | |
72 | + topic2.update_ranking | |
73 | + assert_equal [proposal1.abstract, proposal2.abstract], topic.ranking.map(&:abstract) | |
74 | + assert_equal [proposal3.abstract], topic2.ranking.map(&:abstract) | |
75 | + end | |
76 | + | |
77 | + should 'update ranking for a topic' do | |
78 | + proposal1 = ProposalsDiscussionPlugin::Proposal.create!(:parent => topic, :profile => profile, :name => "proposal1", :abstract => 'abstract') | |
79 | + proposal2 = ProposalsDiscussionPlugin::Proposal.create!(:parent => topic, :profile => profile, :name => "proposal2", :abstract => 'abstract') | |
80 | + topic.update_ranking | |
81 | + topic.update_ranking | |
82 | + assert_equal [proposal1.abstract, proposal2.abstract], topic.ranking.map(&:abstract) | |
83 | + end | |
84 | + | |
65 | 85 | end | ... | ... |