Merge pull request #346 from AyuntamientoMadrid/ancestry

Ancestry

Gemfile

@@ -27,7 +27,7 @@ gem 'omniauth-facebook'
 gem 'omniauth-google-oauth2'
 
 gem 'kaminari'
-gem 'acts_as_commentable_with_threading'
+gem 'ancestry'
 gem 'acts-as-taggable-on'
 gem "responders"
 gem 'foundation-rails'

Gemfile.lock

@@ -38,10 +38,6 @@ GEM
       tzinfo (~> 1.1)
     acts-as-taggable-on (3.5.0)
       activerecord (>= 3.2, < 5)
-    acts_as_commentable_with_threading (2.0.0)
-      activerecord (>= 4.0)
-      activesupport (>= 4.0)
-      awesome_nested_set (>= 3.0)
     acts_as_votable (0.10.0)
     addressable (2.3.8)
     ahoy_matey (1.2.1)
@@ -57,9 +53,9 @@ GEM
     akami (1.3.1)
       gyoku (>= 0.4.0)
       nokogiri
+    ancestry (2.1.0)
+      activerecord (>= 3.0.0)
     arel (6.0.3)
-    awesome_nested_set (3.0.2)
-      activerecord (>= 4.0.0, < 5)
     bcrypt (3.1.10)
     binding_of_caller (0.7.2)
       debug_inspector (>= 0.0.1)
@@ -392,9 +388,9 @@ PLATFORMS
 
 DEPENDENCIES
   acts-as-taggable-on
-  acts_as_commentable_with_threading
   acts_as_votable
   ahoy_matey (~> 1.2.1)
+  ancestry
   bullet
   byebug
   cancancan

app/controllers/comments_controller.rb

@@ -1,15 +1,13 @@
 class CommentsController < ApplicationController
   before_action :authenticate_user!
+  before_action :load_commentable, only: :create
   before_action :build_comment, only: :create
-  before_action :parent, only: :create
 
   load_and_authorize_resource
   respond_to :html, :js
 
   def create
     if @comment.save
-      @comment.move_to_child_of(parent) if reply?
-
       Mailer.comment(@comment).deliver_now if email_on_debate_comment?
       Mailer.reply(@comment).deliver_now if email_on_comment_reply?
     else
@@ -37,11 +35,11 @@ class CommentsController < ApplicationController
   private
 
     def comment_params
-      params.require(:comment).permit(:commentable_type, :commentable_id, :body, :as_moderator, :as_administrator)
+      params.require(:comment).permit(:commentable_type, :commentable_id, :parent_id, :body, :as_moderator, :as_administrator)
     end
 
     def build_comment
-      @comment = Comment.build(debate, current_user, comment_params[:body])
+      @comment = Comment.build(@commentable, current_user, comment_params[:body], comment_params[:parent_id].presence)
       check_for_special_comments
     end
 
@@ -53,24 +51,16 @@ class CommentsController < ApplicationController
       end
     end
 
-    def debate
-      @debate ||= Debate.find(params[:debate_id])
-    end
-
-    def parent
-      @parent ||= Comment.find_parent(comment_params)
-    end
-
-    def reply?
-      parent.class == Comment
+    def load_commentable
+      @commentable = Comment.find_commentable(comment_params[:commentable_type], comment_params[:commentable_id])
     end
 
     def email_on_debate_comment?
-      @comment.debate.author.email_on_debate_comment?
+      @comment.commentable.author.email_on_debate_comment?
     end
 
     def email_on_comment_reply?
-      reply? && parent.author.email_on_comment_reply?
+      @comment.reply? && @comment.parent.author.email_on_comment_reply?
     end
 
     def administrator_comment?

app/controllers/debates_controller.rb

@@ -16,9 +16,11 @@ class DebatesController < ApplicationController
 
   def show
     set_debate_votes(@debate)
-    @comments = @debate.root_comments.recent.page(params[:page]).for_render
-    # TODO limit this list to the paginated root comment's children once we have ancestry
-    all_visible_comments = @debate.comment_threads
+    @commentable = @debate
+    @root_comments = @debate.comments.roots.recent.page(params[:page]).per(10).for_render
+    @comments = @root_comments.inject([]){|all, root| all + root.descendants}
+
+    all_visible_comments = @root_comments + @comments
     set_comment_flags(all_visible_comments)
   end
 

app/helpers/comments_helper.rb

@@ -1,11 +1,20 @@
 module CommentsHelper
 
-  def comment_link_text(parent)
-    parent.class == Debate ? t("comments_helper.comment_link") : t("comments_helper.reply_link")
+  def comment_link_text(parent_id)
+    parent_id.present? ? t("comments_helper.reply_link") : t("comments_helper.comment_link")
   end
 
-  def comment_button_text(parent)
-    parent.class == Debate ? t("comments_helper.comment_button") : t("comments_helper.reply_button")
+  def comment_button_text(parent_id)
+    parent_id.present?  ? t("comments_helper.reply_button") : t("comments_helper.comment_button")
+  end
+
+  def parent_or_commentable_dom_id(parent_id, commentable)
+    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}
   end
 
 end

app/models/comment.rb

@@ -1,13 +1,14 @@
 class Comment < ActiveRecord::Base
-  acts_as_nested_set scope: [:commentable_id, :commentable_type], counter_cache: :children_count
   acts_as_paranoid column: :hidden_at
   include ActsAsParanoidAliases
   acts_as_votable
+  has_ancestry
 
   attr_accessor :as_moderator, :as_administrator
 
   validates :body, presence: true
   validates :user, presence: true
+  validates_inclusion_of :commentable_type, in: ["Debate"]
 
   belongs_to :commentable, -> { with_hidden }, polymorphic: true, counter_cache: true
   belongs_to :user, -> { with_hidden }
@@ -23,14 +24,15 @@ class Comment < ActiveRecord::Base
 
   scope :for_render, -> { with_hidden.includes(user: :organization) }
 
-  def self.build(commentable, user, body)
+  def self.build(commentable, user, body, p_id=nil)
     new commentable: commentable,
         user_id:     user.id,
-        body:        body
+        body:        body,
+        parent_id:   p_id
   end
 
-  def self.find_parent(params)
-    params[:commentable_type].constantize.find(params[:commentable_id])
+  def self.find_commentable(c_type, c_id)
+    c_type.constantize.find(c_id)
   end
 
   def debate
@@ -77,16 +79,12 @@ class Comment < ActiveRecord::Base
     moderator_id.present?
   end
 
-  # TODO: faking counter cache since there is a bug with acts_as_nested_set :counter_cache
-  # Remove when https://github.com/collectiveidea/awesome_nested_set/issues/294 is fixed
-  # and reset counters using
-  # > Comment.find_each { |comment| Comment.reset_counters(comment.id, :children) }
-  def children_count
-    children.count
+  def after_hide
+    commentable_type.constantize.reset_counters(commentable_id, :comments)
   end
 
-  def after_hide
-    commentable_type.constantize.reset_counters(commentable_id, :comment_threads)
+  def reply?
+    !root?
   end
 
 end

app/models/debate.rb

@@ -6,13 +6,13 @@ class Debate < ActiveRecord::Base
   TITLE_LENGTH = Debate.columns.find { |c| c.name == 'title' }.limit
 
   acts_as_votable
-  acts_as_commentable
   acts_as_taggable
   acts_as_paranoid column: :hidden_at
   include ActsAsParanoidAliases
 
   belongs_to :author, -> { with_hidden }, class_name: 'User', foreign_key: 'author_id'
   has_many :flags, :as => :flaggable
+  has_many :comments, as: :commentable
 
   validates :title, presence: true
   validates :description, presence: true

app/views/comments/_comment.html.erb

@@ -2,7 +2,7 @@
 	<div id="<%= dom_id(comment) %>" class="comment small-12 column">
 
     <% if comment.hidden? || comment.user.hidden? %>
-      <% if comment.children_count > 0 %>
+      <% if select_children(@comments, comment).size > 0 %>
         <div class="is-deleted">
           <p><%= t("debates.comment.deleted") %></p>
         </div>
@@ -49,7 +49,7 @@
                 <%= t("shared.collective") %>
               </span>
             <% end %>
-            <% if comment.user_id == @debate.author_id %>
+            <% if comment.user_id == @commentable.author_id %>
               &nbsp;&bullet;&nbsp;
               <span class="label round is-author">
                 <%= t("debates.comment.author") %>
@@ -65,11 +65,11 @@
           <p class="comment-user is-admin"><%= comment.body %></p>
         <% elsif comment.as_moderator? %>
           <p class="comment-user is-moderator"><%= comment.body %></p>
-        <% elsif comment.user.official? && comment.user_id == @debate.author_id %>
+        <% elsif comment.user.official? && comment.user_id == @commentable.author_id %>
           <p class="comment-user level-<%= comment.user.official_level %> is-author"><%= comment.body %></p>
         <% elsif comment.user.official? %>
           <p class="comment-user level-<%= comment.user.official_level %>"><%= comment.body %></p>
-        <% elsif comment.user_id == @debate.author_id %>
+        <% elsif comment.user_id == @commentable.author_id %>
           <p class="comment-user is-author"><%= comment.body %></p>
         <% else %>
           <p class="comment-user"><%= comment.body %></p>
@@ -79,7 +79,7 @@
         </span>
 
         <div class="reply">
-          <%= t("debates.comment.responses", count: comment.children_count) %>
+          <%= t("debates.comment.responses", count: select_children(@comments, comment).size) %>
 
           <% if user_signed_in? %>
             <span class="divider">&nbsp;|&nbsp;</span>
@@ -88,14 +88,14 @@
 
             <%= render 'comments/actions', comment: comment %>
 
-            <%= render 'comments/form', {parent: comment, toggeable: true} %>
+            <%= render 'comments/form', {commentable: @commentable, parent_id: comment.id, toggeable: true} %>
           <% end %>
         </div>
       </div>
     <% end %>
 
     <div class="comment-children">
-      <%= render comment.children.for_render.reorder('id DESC, lft') %>
+      <%= render select_children(@comments, comment) if @comments.present? %>
     </div>
   </div>
 </div>

app/views/comments/_form.html.erb

@@ -1,22 +1,24 @@
-<div id="js-comment-form-<%= dom_id(parent) %>" <%= "style='display:none'".html_safe if toggeable %>>
-  <%= form_for [@debate, Comment.new], remote: true do |f| %>
-    <%= label_tag "comment-body-#{dom_id(parent)}", t("comments.form.leave_comment") %>
-    <%= f.text_area :body, id: "comment-body-#{dom_id(parent)}", label: false %>
-    <%= f.hidden_field :commentable_type, value: parent.class %>
-    <%= f.hidden_field :commentable_id, value: parent.id %>
+<% css_id = parent_or_commentable_dom_id(parent_id, commentable) %>
+<div id="js-comment-form-<%= css_id %>" <%= "style='display:none'".html_safe if toggeable %>>
+  <%= form_for [commentable, Comment.new], remote: true do |f| %>
+    <%= label_tag "comment-body-#{css_id}", t("comments.form.leave_comment") %>
+    <%= f.text_area :body, id: "comment-body-#{css_id}", label: false %>
+    <%= f.hidden_field :commentable_type, value: commentable.class.name %>
+    <%= f.hidden_field :commentable_id, value: commentable.id %>
+    <%= f.hidden_field :parent_id, value: parent_id %>
 
-    <%= f.submit comment_button_text(parent), class: "button radius small inline-block" %>
+    <%= f.submit comment_button_text(parent_id), class: "button radius small inline-block" %>
 
-    <% if can? :comment_as_moderator, @debate %>
+    <% if can? :comment_as_moderator, commentable %>º
       <div class="right">
-        <%= f.check_box :as_moderator, id: "comment-as-moderator-#{dom_id(parent)}", label: false %>
-        <%= label_tag "comment-as-moderator-#{dom_id(parent)}", t("comments.form.comment_as_moderator"), class: "checkbox" %>
+        <%= f.check_box :as_moderator, id: "comment-as-moderator-#{css_id}", label: false %>
+        <%= label_tag "comment-as-moderator-#{css_id}", t("comments.form.comment_as_moderator"), class: "checkbox" %>
       </div>
     <% end %>
-    <% if can? :comment_as_administrator, @debate %>
+    <% if can? :comment_as_administrator, commentable %>
       <div class="right">
-        <%= f.check_box :as_administrator, id: "comment-as-administrator-#{dom_id(parent)}",label: false %>
-        <%= label_tag "comment-as-administrator-#{dom_id(parent)}", t("comments.form.comment_as_admin"), class: "checkbox" %>
+        <%= f.check_box :as_administrator, id: "comment-as-administrator-#{css_id}",label: false %>
+        <%= label_tag "comment-as-administrator-#{css_id}", t("comments.form.comment_as_admin"), class: "checkbox" %>
       </div>
     <% end %>
   <% end %>

app/views/comments/create.js.erb

@@ -1,10 +1,11 @@
-var parent_id = "<%= dom_id(@parent) %>";
 var comment_html = "<%= j(render @comment) %>"
 
-<% if @parent.is_a?(Debate) -%>
-  App.Comments.reset_form(parent_id);
-  App.Comments.add_comment(parent_id, comment_html);
+<% if @comment.root? -%>
+  var commentable_id = '<%= dom_id(@commentable) %>';
+  App.Comments.reset_form(commentable_id);
+  App.Comments.add_comment(commentable_id, comment_html);
 <% else -%>
+  var parent_id = '<%= "comment_#{@comment.parent_id}" %>';
   App.Comments.reset_and_hide_form(parent_id);
   App.Comments.add_reply(parent_id, comment_html);
 <% end -%>

app/views/comments/new.js.erb

@@ -1,2 +1,5 @@
-var field_with_errors = "#js-comment-form-<%= dom_id(@parent) %> #comment-body-<%= dom_id(@parent) %>";
+<% dom_id = parent_or_commentable_dom_id(@comment.parent_id, @commentable) %>
+
+var field_with_errors = "#js-comment-form-<%= dom_id %> #comment-body-<%= dom_id %>";
 App.Comments.display_error(field_with_errors, "<%= j render('comments/errors') %>");
+

app/views/debates/show.html.erb

@@ -82,7 +82,7 @@
         <span>(<%= @debate.comments_count %>)</span>
       </h2>
       <% if user_signed_in? %>
-          <%= render 'comments/form', {parent: @debate, toggeable: false} %>
+          <%= render 'comments/form', {commentable: @debate, parent_id: nil, toggeable: false} %>
       <% else %>
         <br>
         <div class="alert-box radius info">
@@ -92,8 +92,8 @@
         </div>
       <% end %>
 
-      <%= render @comments %>
-      <%= paginate @comments %>
+      <%= render @root_comments %>
+      <%= paginate @root_comments %>
     </div>
   </div>
 </section>

db/dev_seeds.rb

@@ -85,8 +85,8 @@ puts "Commenting Comments"
   comment = Comment.create!(user: author,
                   commentable_id: parent.commentable_id,
                   commentable_type: parent.commentable_type,
-                  body: Faker::Lorem.sentence)
-  comment.move_to_child_of(parent)
+                  body: Faker::Lorem.sentence,
+                  parent: parent)
 end
 
 

db/migrate/20150902114558_add_ancestry_to_comments.rb

@@ -0,0 +1,7 @@
+class AddAncestryToComments < ActiveRecord::Migration
+  def change
+    add_column :comments, :ancestry, :string
+
+    add_index :comments, :ancestry
+  end
+end

db/migrate/20150902120006_remove_parent_id_from_comments.rb

@@ -0,0 +1,9 @@
+class RemoveParentIdFromComments < ActiveRecord::Migration
+  def change
+    Comment.build_ancestry_from_parent_ids! rescue nil
+
+    remove_column :comments, :parent_id, :integer
+    remove_column :comments, :lft, :integer
+    remove_column :comments, :rgt, :integer
+  end
+end

db/migrate/20150903142924_remove_children_count_from_comments.rb

@@ -0,0 +1,5 @@
+class RemoveChildrenCountFromComments < ActiveRecord::Migration
+  def change
+    remove_column :comments, :children_count, :integer, default: 0
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20150902191315) do
+ActiveRecord::Schema.define(version: 20150903142924) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -62,9 +62,6 @@ ActiveRecord::Schema.define(version: 20150902191315) do
     t.text     "body"
     t.string   "subject"
     t.integer  "user_id",                        null: false
-    t.integer  "parent_id"
-    t.integer  "lft"
-    t.integer  "rgt"
     t.datetime "created_at"
     t.datetime "updated_at"
     t.datetime "hidden_at"
@@ -77,8 +74,10 @@ ActiveRecord::Schema.define(version: 20150902191315) do
     t.integer  "cached_votes_up",    default: 0
     t.integer  "cached_votes_down",  default: 0
     t.datetime "confirmed_hide_at"
+    t.string   "ancestry"
   end
 
+  add_index "comments", ["ancestry"], name: "index_comments_on_ancestry", using: :btree
   add_index "comments", ["cached_votes_down"], name: "index_comments_on_cached_votes_down", using: :btree
   add_index "comments", ["cached_votes_total"], name: "index_comments_on_cached_votes_total", using: :btree
   add_index "comments", ["cached_votes_up"], name: "index_comments_on_cached_votes_up", using: :btree

spec/features/comments_spec.rb

@@ -21,7 +21,7 @@ feature 'Comments' do
 
   scenario 'Paginated comments' do
     debate = create(:debate)
-    per_page = Kaminari.config.default_per_page
+    per_page = 10
     (per_page + 2).times { create(:comment, commentable: debate)}
 
     visit debate_path(debate)
@@ -124,8 +124,7 @@ feature 'Comments' do
     parent = create(:comment, commentable: debate)
 
     7.times do
-      create(:comment, commentable: debate).
-      move_to_child_of(parent)
+      create(:comment, commentable: debate, parent: parent)
       parent = parent.children.first
     end
 

spec/models/comment_spec.rb

@@ -17,37 +17,6 @@ describe Comment do
     expect(debate.reload.comments_count).to eq(0)
   end
 
-  describe "#children_count" do
-    let(:comment) { create(:comment) }
-    let(:debate)  { comment.debate }
-
-    it "should count first level children" do
-      parent = comment
-
-      3.times do
-        create(:comment, commentable: debate).
-        move_to_child_of(parent)
-        parent = parent.children.first
-      end
-
-      expect(comment.children_count).to eq(1)
-      expect(debate.comment_threads.count).to eq(4)
-    end
-
-    it "should increase children count" do
-      expect do
-        create(:comment, commentable: debate).
-        move_to_child_of(comment)
-      end.to change { comment.children_count }.from(0).to(1)
-    end
-
-    it "should decrease children count" do
-      new_comment = create(:comment, commentable: debate).move_to_child_of(comment)
-
-      expect { new_comment.destroy }.to change { comment.children_count }.from(1).to(0)
-    end
-  end
-
   describe "#as_administrator?" do
     it "should be true if comment has administrator_id, false otherway" do
       expect(comment).not_to be_as_administrator

spec/support/common_actions.rb

@@ -34,13 +34,14 @@ module CommonActions
   end
 
   def comment_on(debate)
-    user2 = create(:user)
+    user = create(:user)
 
-    login_as(user2)
+    login_as(user)
     visit debate_path(debate)
 
     fill_in "comment-body-debate_#{debate.id}", with: 'Have you thought about...?'
     click_button 'Publish comment'
+
     expect(page).to have_content 'Have you thought about...?'
   end