diff --git a/app/assets/stylesheets/layout.scss b/app/assets/stylesheets/layout.scss index 291faf49d..2a78d3cc7 100644 --- a/app/assets/stylesheets/layout.scss +++ b/app/assets/stylesheets/layout.scss @@ -1958,6 +1958,25 @@ table { opacity: 0.4; } +.wide-order-selector { + float: none; + margin-top: 0; + + @media (min-width: $small-breakpoint) { + float: right; + margin-top: rem-calc(-24); + } + + label { + padding-right: rem-calc(12); + float: none; + + @media (min-width: $small-breakpoint) { + float: right; + } + } +} + // 19. Flags // - - - - - - - - - - - - - - - - - - - - - - - - - @@ -1995,4 +2014,4 @@ table { overflow: hidden; clip: rect(0, 0, 0, 0); border: 0; -} \ No newline at end of file +} diff --git a/app/controllers/concerns/commentable_actions.rb b/app/controllers/concerns/commentable_actions.rb index a6ddf7b57..9eb27226b 100644 --- a/app/controllers/concerns/commentable_actions.rb +++ b/app/controllers/concerns/commentable_actions.rb @@ -16,11 +16,8 @@ module CommentableActions def show set_resource_votes(resource) @commentable = resource - @root_comments = resource.comments.roots.recent.page(params[:page]).per(10).for_render - @comments = @root_comments.inject([]){|all, root| all + Comment.descendants_of(root).for_render} - @all_visible_comments = @root_comments + @comments - - set_comment_flags(@all_visible_comments) + @comment_tree = CommentTree.new(@commentable, params[:page], @current_order) + set_comment_flags(@comment_tree.comments) set_resource_instance end @@ -91,4 +88,4 @@ module CommentableActions def index_customization nil end -end \ No newline at end of file +end diff --git a/app/controllers/debates_controller.rb b/app/controllers/debates_controller.rb index 7981fb121..8a5def88b 100644 --- a/app/controllers/debates_controller.rb +++ b/app/controllers/debates_controller.rb @@ -7,6 +7,7 @@ class DebatesController < ApplicationController before_action :authenticate_user!, except: [:index, :show] has_orders %w{hot_score confidence_score created_at most_commented random}, only: :index + has_orders %w{most_voted newest oldest}, only: :show load_and_authorize_resource respond_to :html, :js diff --git a/app/controllers/management/proposals_controller.rb b/app/controllers/management/proposals_controller.rb index e6fcc69ea..4d96af093 100644 --- a/app/controllers/management/proposals_controller.rb +++ b/app/controllers/management/proposals_controller.rb @@ -7,6 +7,7 @@ class Management::ProposalsController < Management::BaseController before_action :parse_search_terms, only: :index has_orders %w{confidence_score hot_score created_at most_commented random}, only: [:index, :print] + has_orders %w{most_voted newest}, only: :show def vote @proposal.register_vote(current_user, 'yes') diff --git a/app/controllers/moderation/comments_controller.rb b/app/controllers/moderation/comments_controller.rb index cf3197383..d5fbf7954 100644 --- a/app/controllers/moderation/comments_controller.rb +++ b/app/controllers/moderation/comments_controller.rb @@ -2,7 +2,7 @@ class Moderation::CommentsController < Moderation::BaseController include ModerateActions has_filters %w{pending_flag_review all with_ignored_flag}, only: :index - has_orders %w{flags created_at}, only: :index + has_orders %w{flags newest}, only: :index before_action :load_resources, only: [:index, :moderate] diff --git a/app/controllers/proposals_controller.rb b/app/controllers/proposals_controller.rb index 81bbc7c01..1be046b7a 100644 --- a/app/controllers/proposals_controller.rb +++ b/app/controllers/proposals_controller.rb @@ -7,6 +7,7 @@ class ProposalsController < ApplicationController before_action :authenticate_user!, except: [:index, :show] has_orders %w{hot_score confidence_score created_at most_commented random}, only: :index + has_orders %w{most_voted newest oldest}, only: :show load_and_authorize_resource respond_to :html, :js diff --git a/app/helpers/comments_helper.rb b/app/helpers/comments_helper.rb index 0f8eca926..9ac29e4da 100644 --- a/app/helpers/comments_helper.rb +++ b/app/helpers/comments_helper.rb @@ -12,9 +12,9 @@ module CommentsHelper parent_id.blank? ? dom_id(commentable) : "comment_#{parent_id}" end - def select_children(comments, parent) - return [] if comments.blank? - comments.select{|c| c.parent_id == parent.id} + def child_comments_of(parent) + return [] unless @comment_tree + @comment_tree.children_of(parent) end def user_level_class(comment) diff --git a/app/models/comment.rb b/app/models/comment.rb index e9bb59775..f486850a6 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -17,11 +17,20 @@ class Comment < ActiveRecord::Base belongs_to :commentable, -> { with_hidden }, polymorphic: true, counter_cache: true belongs_to :user, -> { with_hidden } - scope :recent, -> { order(id: :desc) } + before_save :calculate_confidence_score + scope :for_render, -> { with_hidden.includes(user: :organization) } scope :with_visible_author, -> { joins(:user).where("users.hidden_at IS NULL") } scope :sort_by_flags, -> { order(flags_count: :desc, updated_at: :desc) } - scope :sort_by_created_at, -> { order(created_at: :desc) } + + scope :sort_by_most_voted , -> { order(confidence_score: :desc, created_at: :desc) } + scope :sort_descendants_by_most_voted , -> { order(confidence_score: :desc, created_at: :asc) } + + scope :sort_by_newest, -> { order(created_at: :desc) } + scope :sort_descendants_by_newest, -> { order(created_at: :desc) } + + scope :sort_by_oldest, -> { order(created_at: :asc) } + scope :sort_descendants_by_oldest, -> { order(created_at: :asc) } after_create :call_after_commented @@ -88,6 +97,11 @@ class Comment < ActiveRecord::Base 1000 end + def calculate_confidence_score + self.confidence_score = ScoreCalculator.confidence_score(cached_votes_total, + cached_votes_up) + end + private def validate_body_length diff --git a/app/views/comments/_comment.html.erb b/app/views/comments/_comment.html.erb index c353fb499..1ec571633 100644 --- a/app/views/comments/_comment.html.erb +++ b/app/views/comments/_comment.html.erb @@ -3,7 +3,7 @@
<% if comment.hidden? || comment.user.hidden? %> - <% if select_children(@comments, comment).size > 0 %> + <% if child_comments_of(comment).size > 0 %>

<%= t("comments.comment.deleted") %>

@@ -73,7 +73,7 @@
- <%= t("comments.comment.responses", count: select_children(@comments, comment).size) %> + <%= t("comments.comment.responses", count: child_comments_of(comment).size) %> <% if user_signed_in? %>  |  @@ -88,7 +88,7 @@
<% end %>
- <% select_children(@comments, comment).each do |child| %> + <% child_comments_of(comment).each do |child| %> <%= render 'comments/comment', comment: child %> <% end %>
diff --git a/app/views/debates/_comments.html.erb b/app/views/debates/_comments.html.erb index 8aa26ed03..538847a62 100644 --- a/app/views/debates/_comments.html.erb +++ b/app/views/debates/_comments.html.erb @@ -1,4 +1,4 @@ -<% cache [locale_and_user_status, commentable_cache_key(@debate), @all_visible_comments, @all_visible_comments.map(&:author), @debate.comments_count, @comment_flags] do %> +<% cache [locale_and_user_status, @current_order, commentable_cache_key(@debate), @comment_tree.comments, @comment_tree.comment_authors, @debate.comments_count, @comment_flags] do %>
@@ -7,6 +7,8 @@ (<%= @debate.comments_count %>) + <%= render 'shared/wide_order_selector', i18n_namespace: "comments" %> + <% if user_signed_in? %> <%= render 'comments/form', {commentable: @debate, parent_id: nil, toggeable: false} %> <% else %> @@ -19,11 +21,11 @@
<% end %> - <% @root_comments.each do |comment| %> + <% @comment_tree.root_comments.each do |comment| %> <%= render 'comments/comment', comment: comment %> <% end %> - <%= paginate @root_comments %> + <%= paginate @comment_tree.root_comments %>
-<% end %> \ No newline at end of file +<% end %> diff --git a/app/views/proposals/_comments.html.erb b/app/views/proposals/_comments.html.erb index 2442bd230..eb54f9796 100644 --- a/app/views/proposals/_comments.html.erb +++ b/app/views/proposals/_comments.html.erb @@ -1,12 +1,15 @@ -<% cache [locale_and_user_status, commentable_cache_key(@proposal), @all_visible_comments, @all_visible_comments.map(&:author), @proposal.comments_count, @comment_flags] do %> +<% cache [locale_and_user_status, @current_order, commentable_cache_key(@proposal), @comment_tree.comments, @comment_tree.comment_authors, @proposal.comments_count, @comment_flags] do %>
+

<%= t("proposals.show.comments_title") %> (<%= @proposal.comments_count %>)

+ <%= render 'shared/wide_order_selector', i18n_namespace: "comments" %> + <% if user_signed_in? %> <%= render 'comments/form', {commentable: @proposal, parent_id: nil, toggeable: false} %> <% else %> @@ -19,11 +22,11 @@
<% end %> - <% @root_comments.each do |comment| %> + <% @comment_tree.root_comments.each do |comment| %> <%= render 'comments/comment', comment: comment %> <% end %> - <%= paginate @root_comments %> + <%= paginate @comment_tree.root_comments %>
-<% end %> \ No newline at end of file +<% end %> diff --git a/app/views/shared/_wide_order_selector.html.erb b/app/views/shared/_wide_order_selector.html.erb new file mode 100644 index 000000000..defa8415e --- /dev/null +++ b/app/views/shared/_wide_order_selector.html.erb @@ -0,0 +1,24 @@ +<% # Params: + # + # i18n_namespace: for example "moderation.debates.index" +%> + +
+
+
+ +
+
+ +
+
+
diff --git a/config/locales/en.yml b/config/locales/en.yml index 04341b299..2d963f3be 100755 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -239,6 +239,11 @@ en: form: submit_button: "Save changes" comments: + select_order: "Sort by" + orders: + most_voted: "Most voted" + newest: "Newest first" + oldest: "Oldest first" form: leave_comment: "Leave your comment" comment_as_moderator: "Comment as moderator" diff --git a/config/locales/es.yml b/config/locales/es.yml index 60a557908..34871cce8 100644 --- a/config/locales/es.yml +++ b/config/locales/es.yml @@ -239,6 +239,11 @@ es: form: submit_button: "Guardar cambios" comments: + select_order: "Ordenar por" + orders: + most_voted: "Más votados" + newest: "Más nuevos primero" + oldest: "Más antiguos primero" form: leave_comment: "Deja tu comentario" comment_as_moderator: "Comentar como moderador" diff --git a/config/locales/moderation.en.yml b/config/locales/moderation.en.yml index b66ffd136..5c85ebf69 100755 --- a/config/locales/moderation.en.yml +++ b/config/locales/moderation.en.yml @@ -24,7 +24,7 @@ en: with_ignored_flag: "Marked as viewed" order: "Order" orders: - created_at: "Newest" + newest: "Newest" flags: "Most flagged" confirm: "Are you sure?" debates: diff --git a/config/locales/moderation.es.yml b/config/locales/moderation.es.yml index 6b4a64692..d99a48877 100644 --- a/config/locales/moderation.es.yml +++ b/config/locales/moderation.es.yml @@ -24,7 +24,7 @@ es: with_ignored_flag: "Marcados como revisados" order: "Orden" orders: - created_at: "Más nuevos" + newest: "Más nuevos" flags: "Más denunciados" confirm: "¿Estás seguro?" debates: diff --git a/db/migrate/20151028145921_add_confidence_score_to_comments.rb b/db/migrate/20151028145921_add_confidence_score_to_comments.rb new file mode 100644 index 000000000..636b648bb --- /dev/null +++ b/db/migrate/20151028145921_add_confidence_score_to_comments.rb @@ -0,0 +1,5 @@ +class AddConfidenceScoreToComments < ActiveRecord::Migration + def change + add_column :comments, :confidence_score, :integer, index: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 823406cbf..cbe2fd671 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -75,6 +75,7 @@ ActiveRecord::Schema.define(version: 20151030182217) do t.integer "cached_votes_down", default: 0 t.datetime "confirmed_hide_at" t.string "ancestry" + t.integer "confidence_score" end add_index "comments", ["ancestry"], name: "index_comments_on_ancestry", using: :btree diff --git a/lib/comment_tree.rb b/lib/comment_tree.rb new file mode 100644 index 000000000..8d0e072dd --- /dev/null +++ b/lib/comment_tree.rb @@ -0,0 +1,28 @@ +class CommentTree + + ROOT_COMMENTS_PER_PAGE = 10 + + attr_accessor :root_comments, :comments + + def initialize(commentable, page, order = 'confidence_score') + @root_comments = commentable.comments.roots.send("sort_by_#{order}").page(page).per(ROOT_COMMENTS_PER_PAGE).for_render + + root_descendants = @root_comments.each_with_object([]) do |root, col| + col.concat(Comment.descendants_of(root).send("sort_descendants_by_#{order}").for_render.to_a) + end + + @comments = root_comments + root_descendants + + @comments_by_parent_id = @comments.each_with_object({}) do |comment, col| + (col[comment.parent_id] ||= []) << comment + end + end + + def children_of(parent) + @comments_by_parent_id[parent.id] || [] + end + + def comment_authors + comments.map(&:author) + end +end diff --git a/lib/tasks/comments.rake b/lib/tasks/comments.rake index 6c3f32687..61f137156 100644 --- a/lib/tasks/comments.rake +++ b/lib/tasks/comments.rake @@ -6,4 +6,11 @@ namespace :comments do Proposal.all.pluck(:id).each{ |id| Proposal.reset_counters(id, :comments) } end + desc "Recalculates all the comment confidence scores (used for sorting comments)" + task confidence_score: :environment do + Comment.find_in_batches do |comments| + comments.each(&:save) + end + end + end diff --git a/spec/factories.rb b/spec/factories.rb index eff656edf..878627e0d 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -197,7 +197,7 @@ FactoryGirl.define do factory :comment do association :commentable, factory: :debate user - body 'Comment body' + sequence(:body) { |n| "Comment body #{n}" } trait :hidden do hidden_at Time.now @@ -216,6 +216,10 @@ FactoryGirl.define do Flag.flag(FactoryGirl.create(:user), debate) end end + + trait :with_confidence_score do + before(:save) { |d| d.calculate_confidence_score } + end end factory :administrator do diff --git a/spec/features/comments/debates_spec.rb b/spec/features/comments/debates_spec.rb index 073e9fa5c..ca9a33eb8 100644 --- a/spec/features/comments/debates_spec.rb +++ b/spec/features/comments/debates_spec.rb @@ -20,6 +20,49 @@ feature 'Commenting debates' do end end + scenario 'Comment order' do + c1 = create(:comment, :with_confidence_score, commentable: debate, cached_votes_up: 100, cached_votes_total: 120, created_at: Time.now - 2) + c2 = create(:comment, :with_confidence_score, commentable: debate, cached_votes_up: 10, cached_votes_total: 12, created_at: Time.now - 1) + c3 = create(:comment, :with_confidence_score, commentable: debate, cached_votes_up: 1, cached_votes_total: 2, created_at: Time.now) + + visit debate_path(debate, order: :most_voted) + + expect(c1.body).to appear_before(c2.body) + expect(c2.body).to appear_before(c3.body) + + visit debate_path(debate, order: :newest) + + expect(c3.body).to appear_before(c2.body) + expect(c2.body).to appear_before(c1.body) + + visit debate_path(debate, order: :oldest) + + expect(c1.body).to appear_before(c2.body) + expect(c2.body).to appear_before(c3.body) + end + + scenario 'Creation date works differently in roots and in child comments, even when sorting by confidence_score' do + old_root = create(:comment, commentable: debate, created_at: Time.now - 10) + new_root = create(:comment, commentable: debate, created_at: Time.now) + old_child = create(:comment, commentable: debate, parent_id: new_root.id, created_at: Time.now - 10) + new_child = create(:comment, commentable: debate, parent_id: new_root.id, created_at: Time.now) + + visit debate_path(debate, order: :most_voted) + + expect(new_root.body).to appear_before(old_root.body) + expect(old_child.body).to appear_before(new_child.body) + + visit debate_path(debate, order: :newest) + + expect(new_root.body).to appear_before(old_root.body) + expect(new_child.body).to appear_before(old_child.body) + + visit debate_path(debate, order: :oldest) + + expect(old_root.body).to appear_before(new_root.body) + expect(old_child.body).to appear_before(new_child.body) + end + scenario 'Turns links into html links' do create :comment, commentable: debate, body: 'Built with http://rubyonrails.org/' @@ -71,7 +114,6 @@ feature 'Commenting debates' do within('#comments') do expect(page).to_not have_content 'Write a comment' expect(page).to_not have_content 'Reply' - expect(page).to_not have_css('form') end end end diff --git a/spec/features/comments/proposals_spec.rb b/spec/features/comments/proposals_spec.rb index 7d51e79e3..1c277cf4d 100644 --- a/spec/features/comments/proposals_spec.rb +++ b/spec/features/comments/proposals_spec.rb @@ -20,6 +20,49 @@ feature 'Commenting proposals' do end end + scenario 'Comment order' do + c1 = create(:comment, :with_confidence_score, commentable: proposal, cached_votes_up: 100, cached_votes_total: 120, created_at: Time.now - 2) + c2 = create(:comment, :with_confidence_score, commentable: proposal, cached_votes_up: 10, cached_votes_total: 12, created_at: Time.now - 1) + c3 = create(:comment, :with_confidence_score, commentable: proposal, cached_votes_up: 1, cached_votes_total: 2, created_at: Time.now) + + visit proposal_path(proposal, order: :most_voted) + + expect(c1.body).to appear_before(c2.body) + expect(c2.body).to appear_before(c3.body) + + visit proposal_path(proposal, order: :newest) + + expect(c3.body).to appear_before(c2.body) + expect(c2.body).to appear_before(c1.body) + + visit proposal_path(proposal, order: :oldest) + + expect(c1.body).to appear_before(c2.body) + expect(c2.body).to appear_before(c3.body) + end + + scenario 'Creation date works differently in roots and in child comments, when sorting by confidence_score' do + old_root = create(:comment, commentable: proposal, created_at: Time.now - 10) + new_root = create(:comment, commentable: proposal, created_at: Time.now) + old_child = create(:comment, commentable: proposal, parent_id: new_root.id, created_at: Time.now - 10) + new_child = create(:comment, commentable: proposal, parent_id: new_root.id, created_at: Time.now) + + visit proposal_path(proposal, order: :most_voted) + + expect(new_root.body).to appear_before(old_root.body) + expect(old_child.body).to appear_before(new_child.body) + + visit proposal_path(proposal, order: :newest) + + expect(new_root.body).to appear_before(old_root.body) + expect(new_child.body).to appear_before(old_child.body) + + visit proposal_path(proposal, order: :oldest) + + expect(old_root.body).to appear_before(new_root.body) + expect(old_child.body).to appear_before(new_child.body) + end + scenario 'Turns links into html links' do create :comment, commentable: proposal, body: 'Built with http://rubyonrails.org/' @@ -71,7 +114,6 @@ feature 'Commenting proposals' do within('#comments') do expect(page).to_not have_content 'Write a comment' expect(page).to_not have_content 'Reply' - expect(page).to_not have_css('form') end end end diff --git a/spec/features/management/proposals_spec.rb b/spec/features/management/proposals_spec.rb index e33c1a179..ffab09f55 100644 --- a/spec/features/management/proposals_spec.rb +++ b/spec/features/management/proposals_spec.rb @@ -197,4 +197,4 @@ feature 'Proposals' do end end -end \ No newline at end of file +end diff --git a/spec/features/moderation/comments_spec.rb b/spec/features/moderation/comments_spec.rb index 1774a1585..4a7249042 100644 --- a/spec/features/moderation/comments_spec.rb +++ b/spec/features/moderation/comments_spec.rb @@ -104,15 +104,15 @@ feature 'Moderate comments' do scenario "remembering page, filter and order" do create_list(:comment, 52) - visit moderation_comments_path(filter: 'all', page: '2', order: 'created_at') + visit moderation_comments_path(filter: 'all', page: '2', order: 'newest') click_on "Mark as viewed" - expect(page).to have_selector('.js-order-selector[data-order="created_at"]') + expect(page).to have_selector('.js-order-selector[data-order="newest"]') expect(current_url).to include('filter=all') expect(current_url).to include('page=2') - expect(current_url).to include('order=created_at') + expect(current_url).to include('order=newest') end end @@ -174,7 +174,7 @@ feature 'Moderate comments' do create(:comment, body: "Flagged newer comment", created_at: Time.now - 12.hours, flags_count: 3) create(:comment, body: "Newer comment", created_at: Time.now) - visit moderation_comments_path(order: 'created_at') + visit moderation_comments_path(order: 'newest') expect("Flagged newer comment").to appear_before("Flagged comment") @@ -182,7 +182,7 @@ feature 'Moderate comments' do expect("Flagged comment").to appear_before("Flagged newer comment") - visit moderation_comments_path(filter: 'all', order: 'created_at') + visit moderation_comments_path(filter: 'all', order: 'newest') expect("Newer comment").to appear_before("Flagged newer comment") expect("Flagged newer comment").to appear_before("Flagged comment") diff --git a/spec/models/comment_spec.rb b/spec/models/comment_spec.rb index 6d37eb71b..cb2c61181 100644 --- a/spec/models/comment_spec.rb +++ b/spec/models/comment_spec.rb @@ -39,6 +39,44 @@ describe Comment do end end + describe "#confidence_score" do + + it "takes into account percentage of total votes and total_positive and total negative votes" do + comment = create(:comment, :with_confidence_score, cached_votes_up: 100, cached_votes_total: 100) + expect(comment.confidence_score).to eq(10000) + + comment = create(:comment, :with_confidence_score, cached_votes_up: 0, cached_votes_total: 100) + expect(comment.confidence_score).to eq(0) + + comment = create(:comment, :with_confidence_score, cached_votes_up: 75, cached_votes_total: 100) + expect(comment.confidence_score).to eq(3750) + + comment = create(:comment, :with_confidence_score, cached_votes_up: 750, cached_votes_total: 1000) + expect(comment.confidence_score).to eq(37500) + + comment = create(:comment, :with_confidence_score, cached_votes_up: 10, cached_votes_total: 100) + expect(comment.confidence_score).to eq(-800) + end + + describe 'actions which affect it' do + let(:comment) { create(:comment, :with_confidence_score) } + + it "increases with like" do + previous = comment.confidence_score + 5.times { comment.vote_by(voter: create(:user), vote: true) } + expect(previous).to be < comment.confidence_score + end + + it "decreases with dislikes" do + comment.vote_by(voter: create(:user), vote: true) + previous = comment.confidence_score + 3.times { comment.vote_by(voter: create(:user), vote: false) } + expect(previous).to be > comment.confidence_score + end + end + + end + describe "cache" do let(:comment) { create(:comment) } @@ -73,4 +111,10 @@ describe Comment do .to change { [comment.reload.updated_at, comment.author.updated_at] } end end + + describe "#author_id?" do + it "returns the user's id" do + expect(comment.author_id).to eq(comment.user.id) + end + end end