From dbcc5fb724501034f0f9638a1e41d7bf7d35eb2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 24 Aug 2018 21:50:58 +0200 Subject: [PATCH] Use AR relations when merging comments Using arrays made it difficult to order by more than one field (like the `most_voted` scope does), and so we were ordering by `confidence_score` and ignoring the `created_at` column. Using an AR relation makes it easy to reuse the existing `most_voted` scope. This change has one side effect: now comments with equal votes are ordered in descending order instead of having no specific order. That means flaky specs which failed sometimes because they assumed comments were ordered by date are now always green. I've also re-added the `oldest` scope removed in 792b15b thinking it was removed because using it with arrays was too hard. --- .../legislation/annotations_controller.rb | 2 +- lib/merged_comment_tree.rb | 17 +++-------------- .../comments/legislation_annotations_spec.rb | 6 +++--- 3 files changed, 7 insertions(+), 18 deletions(-) diff --git a/app/controllers/legislation/annotations_controller.rb b/app/controllers/legislation/annotations_controller.rb index a25248032..7f6e2e663 100644 --- a/app/controllers/legislation/annotations_controller.rb +++ b/app/controllers/legislation/annotations_controller.rb @@ -8,7 +8,7 @@ class Legislation::AnnotationsController < Legislation::BaseController load_and_authorize_resource :draft_version, through: :process load_and_authorize_resource - has_orders %w{most_voted newest}, only: :show + has_orders %w[most_voted newest oldest], only: :show def index @annotations = @draft_version.annotations diff --git a/lib/merged_comment_tree.rb b/lib/merged_comment_tree.rb index efaca5f2f..81916908e 100644 --- a/lib/merged_comment_tree.rb +++ b/lib/merged_comment_tree.rb @@ -1,24 +1,13 @@ class MergedCommentTree < CommentTree - attr_reader :commentables, :array_order + attr_reader :commentables def initialize(commentables, page, order = "confidence_score") @commentables = commentables super(commentables.first, page, order) - @array_order = set_array_order(order) end - def root_comments - Kaminari.paginate_array(commentables.flatten.map(&:comments).map(&:roots).flatten.sort_by {|a| a[array_order]}.reverse). - page(page).per(ROOT_COMMENTS_PER_PAGE) - end - - def set_array_order(order) - case order - when "newest" - "created_at" - else - "confidence_score" - end + def base_comments + Comment.where(commentable: commentables.flatten) end end diff --git a/spec/features/comments/legislation_annotations_spec.rb b/spec/features/comments/legislation_annotations_spec.rb index f226cf529..5b608058a 100644 --- a/spec/features/comments/legislation_annotations_spec.rb +++ b/spec/features/comments/legislation_annotations_spec.rb @@ -14,7 +14,7 @@ describe "Commenting legislation questions" do expect(page).to have_css(".comment", count: 4) - comment = Comment.first + comment = Comment.last within first(".comment") do expect(page).to have_content comment.user.name expect(page).to have_content I18n.l(comment.created_at, format: :datetime) @@ -144,7 +144,7 @@ describe "Commenting legislation questions" do legislation_annotation.draft_version, legislation_annotation) - within all(".comment").last do + within all(".comment").first do expect(page).to have_content "Built with http://rubyonrails.org/" expect(page).to have_link("http://rubyonrails.org/", href: "http://rubyonrails.org/") expect(find_link("http://rubyonrails.org/")[:rel]).to eq("nofollow") @@ -160,7 +160,7 @@ describe "Commenting legislation questions" do legislation_annotation.draft_version, legislation_annotation) - within all(".comment").last do + within all(".comment").first do expect(page).to have_content "click me http://www.url.com" expect(page).to have_link("http://www.url.com", href: "http://www.url.com") expect(page).not_to have_link("click me")