Merge pull request #660 from AyuntamientoMadrid/comments-order

Comments order

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
 // - - - - - - - - - - - - - - - - - - - - - - - - -
 

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
 

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

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')

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]
 

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

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)

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

app/views/comments/_comment.html.erb

@@ -3,7 +3,7 @@
   	<div id="<%= dom_id(comment) %>" class="comment small-12 column">
 
       <% if comment.hidden? || comment.user.hidden? %>
-        <% if select_children(@comments, comment).size > 0 %>
+        <% if child_comments_of(comment).size > 0 %>
           <div class="is-deleted">
             <p><%= t("comments.comment.deleted") %></p>
           </div>
@@ -73,7 +73,7 @@
           </span>
 
           <div class="reply">
-            <%= t("comments.comment.responses", count: select_children(@comments, comment).size) %>
+            <%= t("comments.comment.responses", count: child_comments_of(comment).size) %>
 
             <% if user_signed_in? %>
               <span class="divider">&nbsp;|&nbsp;</span>
@@ -88,7 +88,7 @@
         </div>
       <% end %>
       <div class="comment-children">
-        <% select_children(@comments, comment).each do |child| %>
+        <% child_comments_of(comment).each do |child| %>
           <%= render 'comments/comment', comment: child %>
         <% end %>
       </div>

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 %>
   <section class="row-full comments">
     <div class="row">
       <div id="comments" class="small-12 column">
@@ -7,6 +7,8 @@
           <span class="js-comments-count">(<%= @debate.comments_count %>)</span>
         </h2>
 
+        <%= render 'shared/wide_order_selector', i18n_namespace: "comments" %>
+
         <% if user_signed_in? %>
           <%= render 'comments/form', {commentable: @debate, parent_id: nil, toggeable: false} %>
         <% else %>
@@ -19,10 +21,10 @@
           </div>
         <% 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 %>
       </div>
     </div>
   </section>

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 %>
   <section class="row-full comments">
     <div class="row">
       <div id="comments" class="small-12 column">
+
         <h2>
           <%= t("proposals.show.comments_title") %>
           <span class="js-comments-count">(<%= @proposal.comments_count %>)</span>
         </h2>
 
+        <%= render 'shared/wide_order_selector', i18n_namespace: "comments" %>
+
         <% if user_signed_in? %>
           <%= render 'comments/form', {commentable: @proposal, parent_id: nil, toggeable: false} %>
         <% else %>
@@ -19,10 +22,10 @@
           </div>
         <% 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 %>
       </div>
     </div>
   </section>

app/views/shared/_wide_order_selector.html.erb

@@ -0,0 +1,24 @@
+<% # Params:
+   #
+   #   i18n_namespace: for example "moderation.debates.index"
+%>
+
+<div class="wide-order-selector small-12 medium-8">
+  <form>
+    <div class="small-12 medium-8 left">
+      <label for="order-selector-participation">
+        <%= t("#{i18n_namespace}.select_order") %>
+      </label>
+    </div>
+    <div class="small-12 medium-4 left">
+      <select class="js-location-changer js-order-selector select-order" data-order="<%= @current_order %>" name="order-selector" id="order-selector-participation">
+         <% @valid_orders.each do |order| %>
+           <option <%= 'selected' if order == @current_order %>
+                   value='<%= current_path_with_query_params(order: order, page: 1) %>'>
+             <%= t("#{i18n_namespace}.orders.#{order}") %>
+           </option>
+         <% end %>
+       </select>
+     </div>
+  </form>
+</div>

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"

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"

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:

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:

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

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

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

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

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

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

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

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")

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